mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
feat(user-restriction): make the pending-review mute type a parameter (#4609)
* feat(user-restriction): make the pending-review mute type a parameter
Adds an optional `type` to `applyPendingReviewMute`, defaulting to
'generation' so both existing callers are byte-for-byte unchanged, and
teaches the moderator queue to show a second type. This is the enabling
seam for a bot-account detector that must file into the SAME review
queue rather than a new board; no detector logic ships here.
`UserRestriction.type` is free text with a [type, status] index, so a new
type needs no migration.
Dedupe is now scoped per type. Scoped to the user alone, the first queue
to mute an account would permanently silence every other queue for it:
a later finding of a different kind returns deduped against a row about
something else and files nothing a moderator can see.
Notifications are an opt-in per-type map, with null meaning "say
nothing". createNotification validates its type against nothing, so an
unregistered value is persisted and increments the unread badge while
the bell drops it at render, leaving a phantom count with no click
target; and reusing 'generation-muted' would tell a user their
generation access was restricted for something unrelated to generation.
A new type therefore stays silent until a processor is registered for it.
Verdicts are still generation-shaped, so the moderator resolve and ban
actions refuse a row of any other type rather than sending a misleading
notice. Parameterising that path is deliberately left out of scope.
* test(user-restriction): close two tests that passed against base code
Both were found by running the new suites against pre-change source, and
both were green there for a reason unrelated to what they claim to test.
- The moderator ban refusal asserted only status 400. An invalid ban
payload is also 400 with setBanned untouched, and the fixture used a
reasonCode outside BAN_REASONS — so the schema rejected it before the
type check could run. Uses a valid payload now, asserts the refusal
message, and adds the generation-row positive control that proves the
refusal is not simply rejecting every ban.
- The SELECT-list assertion matched the whole statement, so it was
satisfied by the `"ur"."type" = $1` in the WHERE that every version
emits. Sliced to the select list.
Also fixes a comment naming a function that does not exist.
* fix(user-restriction): refuse an unrulable type inside the verdict path itself
The type refusal added with the queue lived at ONE of the ruling surfaces. Five
callers reach `resolveUserRestriction` — the tRPC router, `/api/mod/restriction/
resolve` (which is what both moderator-app ruling surfaces post through: the
audit queue and the retool User Lookup panel), and `overturnPendingReviewMute` —
and only the audit queue checked. Reaching the verdict path with a non-generation
row would send a "your generation access has been restored" notice and an email,
and call `resetProhibitedRequestCount`, wiping the account's real prompt-violation
counter over an unrelated case.
Moved down rather than replicated at a third route: a predicate open-coded at N
sites is wrong at N-1 of them. `RULINGS_WIRED_FOR` and `unwiredRulingReason` now
live beside the type vocabulary, and the refusal happens in the service, before
any write and before the already-resolved check.
Also validates `type` at runtime in `applyPendingReviewMute`. That seam exists to
accept a caller-supplied type across an HTTP boundary and a JSON body, where the
compiler's word is worth nothing; an out-of-vocabulary value used to mute the
account, file a row no queue can select, and send no notification.
Latent today — one writer, both callers pass no type, so no non-generation row
can exist. Both are closed before a detector ships.
Tests, red at the PR head and green here:
- refuses Overturn and Uphold on a bot-account row, with no write, no
notification, no subscription change and no counter reset
- refuses before it argues about the row's status
- rejects three out-of-vocabulary types with nothing muted and nothing filed
- the seam test pins the moderator app's copy of the wired-for list AND the
refusal wording to this one, in both directions
The fake's `userRestriction.findUnique` now honours `select` for `type` alone, so
a service that refuses non-generation rows but forgets to select the column reads
`undefined` and fails the positive controls instead of passing vacuously.
* fix(moderator): stop the lookup panel hiding an open case, and stop offering rulings that cannot land
Three things, all from the audit on this PR.
1. User Lookup showed the newest restriction of ANY type — three correlated
subqueries, `ORDER BY ur.id DESC LIMIT 1`, no type predicate. That was sound
while a user could hold at most one open row. Per-type dedupe lets two coexist,
so a Pending generation case sitting behind a later Upheld bot-account row
rendered as no open restriction at all: the account stays muted, the ruling
form is never drawn, and nobody can see the open case.
Now `ORDER BY (ur.status = 'Pending') DESC, ur.id DESC` — a Pending case
outranks a merely newer one; among several Pending rows, the newest wins; a
resolved row is shown only when there is no open one. The panel still speaks
for ONE row (a header chip and a single form; the audit queue is where a list
belongs), and the ordering is now written once and called three times rather
than copied per column, so the three cannot stop naming the same row.
2. `RULINGS_WIRED_FOR` / `unwiredRulingReason` move into `$lib/restriction-types`
and are imported by the audit route instead of re-spelled there. The refusal
that protects the account now lives in the main app's `resolveUserRestriction`;
the route check is KEPT, not for defence in depth but for ordering — `ban` bans
and THEN rules, so a refusal arriving inside the verdict call would leave the
account banned against a restriction nobody can close. The seam test pins this
copy to the main app's in both directions, list and wording.
3. The Bot account queue rendered live Uphold / Remove / Ban forms whose only
possible outcome was a 400, and the retool panel offered Overturn / Uphold on a
row the verdict path refuses. Both now disable those controls and say why. The
server-side refusals are unchanged — they hold against a posted id, which
nothing rendered can.
Also: `restrictionById`'s comment claimed `unwiredRuling` governs which types an
action can be handed; `flagSuspicious` calls it with `type: 'any'` and rules on
nothing. The comment now says what the code does and why flagging is deliberately
type-agnostic. And `capturingDb`'s `params` is `unknown[][]`, not a `readonly`
contract cast away at its one use.
Tests: new `user-lookup-restriction-row.test.ts` asserts the Pending-first
ordering, the total tiebreak and the per-account scope on all three compiled
subqueries; new `restriction-types.test.ts` covers the shared predicate the
disabled controls read. The audit route gains an invariant guard that its refusal
is the shared predicate's own output rather than a second copy. There is no
component-test harness in this app, so the rendering half of (3) is not covered by
a test.
* fix(user-restriction): make the seam guard read the moderator vocabulary by executing it
Round-2 audit F1. The guard that pins the two apps' restriction-type lists to each
other read the moderator app's module as TEXT, and it passed green over a real
divergence. Its regex captured to the FIRST closing bracket after the '=', so a
comment naming an index truncated the capture, and it extracted single-quoted
strings only, so a differently-quoted entry vanished. Measured here: with the
moderator list written as
export const RULINGS_WIRED_FOR: readonly RestrictionType[] = [
'generation', // matches RESTRICTION_TYPES[0]
'bot-account',
];
the seam file reported 8 passed / 0 failed while the two lists genuinely
disagreed. The length > 0 positive control could not see it either, because the
first entry survives the truncation. Same green for a mixed-quote entry and for a
list assembled with a spread.
That is not cosmetic. If the moderator app believes a type is rulable, its audit
queue bans the account and posts the ruling afterwards -- a ruling the main app
refuses -- leaving a banned account with a Pending row nobody can close.
Not fixed by a wider regex: a guard that pins source text by PATTERN is walkable
by rewriting the text, and the rewrites that walk it are ordinary (a Prettier
reflow, a comment, a quote style). The reader now IMPORTS AND EXECUTES the module
and compares values, so formatting cannot be the difference between agreeing and
disagreeing. This is available because the moderator vocabulary module has no
imports of its own; a comment there says so and says to keep it that way.
- moderator-restriction-vocabulary.harness.ts: the reader, plus runtime shape
validation that throws (naming the export) rather than returning an empty list.
- moderator-restriction-vocabulary.test.ts: 20 tests. Five fixtures, each a REAL
divergence written in a different shape -- multi-line, a comment containing a
closing bracket, double-quoted entries, a trailing comma, and values assembled
at runtime -- plus seven refusal cases for the shape validation.
- The refusal SENTENCE is now called rather than parsed out of a template
literal, so a message built from constants is compared on what it produces.
- apps/moderator restriction-types.test.ts: pin RULINGS_WIRED_FOR BY VALUE. The
old unwired.length > 0 caught a widened list only by accident -- bot-account
being the only unwired type. Measured: with a third filed type present, the
length check passes over a wrongly-wired bot-account and the value pin is the
only thing that fails.
- .prettierignore: the fixtures' formatting IS the fixture; Prettier would
normalise four distinct cases into one.
Measured, old reader vs new, both run against the same mutated moderator module:
comment containing ']' old 8/8 GREEN new RED (2 failures)
mixed quote style old 8/8 GREEN new RED (2 failures)
spread / computed list old 8/8 GREEN new RED (2 failures)
all-double-quoted old RED (empty-list control) new RED
multi-line, no comment old RED new RED
single-line trailing , old RED new RED
And with the reader reverted to the old text parser, 6 of the 20 new tests fail
(the comment, double-quote and computed fixtures, on both their list and message
cases); the multi-line and trailing-comma fixtures pass under both and are
labelled as declared coverage rather than regression coverage.
* fix(user-restriction): let the ruling refusal survive as a 400, and make two claims true
Round-2 audit F3, F4 and F5.
F4 (behaviour). resolveUserRestriction threw a plain Error, so handleEndpointError
fell to its non-TRPCError branch and the refusal reached the wire as
500 "An unexpected error occurred" -- the moderator's panel rendered
"Restriction ruling: An unexpected error occurred." and the reason was destroyed.
Reachable today from the retool User Lookup panel, which has no local guard.
Now throwBadRequestError, which keeps the status and the message.
Scope decision on that one: the two neighbouring guards in the same function had
the identical defect, and I fixed them in the same change rather than leaving one
of three converted. "Restriction record not found" is now a 404 and "Restriction
has already been resolved" a 400; both are facts about the request, neither is a
server fault, and leaving two of three as opaque 500s would have recreated the
same predicate spelled two ways one line apart. All three are covered.
Covered by three tests that drive the REAL handleEndpointError over the REAL
thrown value, not by asserting the message alone -- a message assertion stays
green through exactly the 500 this fixes. Watched red at ff97751d20:
"expected 500 to be 400", "expected 500 to be 404", "expected 500 to be 400".
F5 (latent). The User Lookup panel's ORDER BY (ur.status = 'Pending') DESC is
correct only while status is NOT NULL: Postgres defaults DESC to NULLS FIRST, so
a NULL would outrank a genuinely Pending row and hide the open case -- the exact
failure the preference exists to prevent, arriving through the column's
nullability. DESC NULLS LAST makes it independent of that. The column is a
NOT NULL enum today, so this is an unstated precondition made explicit, not a
live defect. Two tests red at ff97751d20 on the compiled SQL text.
F3 (comment truth). The runtime type guard's comment claimed the values reaching
it "cross an HTTP boundary and a JSON body". Nothing does: neither production
caller passes a type, and mute-user-pending-review.ts's zod schema has no type
key, so no request body can supply one. The guard stays -- what it is actually
for is the shape of the NEXT caller (this seam exists so a detector can file into
the queue, and the obvious wiring is a route forwarding a JSON field) and the
callers TypeScript cannot vouch for today (an `as` cast, a value read back off
the free-text column, a JS caller). Corrected in the service and in the test
file's docblock, which carried the same false sentence.
* test(user-restriction): close the vocabulary guard's environment blind spot
Round-4 delta audit, F-1/F-3/F-4.
F-1. The execute-based reader resolves the moderator app's vocabulary IN THE
MAIN APP'S TEST PROCESS, so an environment-conditional list is read under
Vitest and never under the moderator app's production build. Reproduced: with
export const RULINGS_WIRED_FOR: readonly RestrictionType[] = import.meta.env.DEV
? ['generation']
: ['generation', 'bot-account'];
the seam + vocabulary suites report 28 passed / 0 failed while the shipped
build carries both types — the ban-then-strand hazard, reached with every
pinning guard green. The base commit's TEXT parser goes RED on that same
module, so the reader that replaced it was not strictly stronger; it traded a
formatting blind spot for a runtime-environment one.
Keeps a TEXT assertion alongside the execute check rather than replacing it:
the module's source may contain no import.meta and no process.env. The two
mechanisms are complementary. With it, the mutant above fails the seam suite at
its beforeAll and the vocabulary suite's real-module case, both naming the
constant.
Comments are stripped before the scan. Without that the guard is matched by its
own documentation — the module has to be able to name the shapes it refuses,
and a raw-text scan fires on the sentence forbidding the thing rather than on
the thing. Covered by a control asserting a commented mention does not trip it.
The moderator module's precondition comment said only 'keep this module
import-free'. import.meta.env and process.env need no import statement, so that
sentence never covered this; it now states both constraints and says they are
separate.
F-3. All five vocabulary fixtures wrote 'return RULINGS_WIRED_FOR.includes(type)'
while the real module writes '(RULINGS_WIRED_FOR as readonly string[])'. The old
parser's message regex requires 'return (RULINGS_WIRED_FOR', so it could not read
the message out of ANY fixture and M8 was measuring the old reader against a
shape the module does not have. Fixtures now carry the cast. Re-measured, M8 is
6 of 21 — the three fixture pairs the body names (comment-with-bracket,
double-quoted, computed), on both their list and message cases. The audit's 8
was the drift; the multi-line-array and trailing-comma fixtures are read
correctly by the old parser again, messages included.
computed.ts's comment now distinguishes 'assembled from constants declared in
this file' — the same value everywhere, which is what the execute reader can
certify — from 'assembled from the environment', which is refused. It was the
fixture that made the hazardous shape look sanctioned.
F-4. The harness docblock now names the cross-app build coupling: resolving the
moderator path makes Vite load apps/moderator/tsconfig.json, which extends the
gitignored generated .svelte-kit/tsconfig.json, so without svelte-kit sync both
suites fail with a TSConfckParseError naming a tsconfig rather than the seam.
CI is unaffected; a fresh clone or worktree is not.
* fix(moderator): read the refusal out of the envelope the endpoint actually sends
Round-4 delta audit, F-2.
handleEndpointError's 4xx pass-through emits { message } and no error key,
while every other refusal from defineModeratorEndpoint emits
{ error, message, code }. The moderator app's readError read body.error and
nothing else, so all three refusals added last round came back null and the
operator saw 'Restriction ruling returned 400.' — the reason destroyed again,
one layer further out than the opaque 500 that change removed.
Fixed at the CONSUMER, not the emitter, and the choice is measured rather than
assumed. handleEndpointError is the shared chokepoint for 36 REST route files;
its 4xx and 503 pass-through bodies are pinned toStrictEqual({ message }) by
endpoint-helpers-error-envelope.test.ts, the 503 case as an explicit documented
carve-out; and restErrorBody needs a RestErrorCode that is not derivable at that
point without a new status-to-code map, which would then have to be reconciled
with the closed key ledger rest-error-envelope-ledger.test.ts enforces. Widening
the reader costs one expression and makes every endpoint's 4xx legible to this
app; widening the emitter changes the wire format of 36 routes and needs its own
PR.
The rule moves to apps/moderator/src/lib/server/rest-error-reason.ts, kept
import-free so the main app's suite can load it by filesystem path — the same
mechanism as the vocabulary harness. That is what lets the new test drive the
REAL emitter into the REAL reader in one process, over all three refusals.
Two suites each mocking the other side is precisely the arrangement that cannot
see a disagreement about a field name, which is why the previous round's tests
were green: they drove the real helper but asserted body.message, which the
consumer never looked at.
Red before the fix with 'expected null not to be null'; the reader's own
null-capability is pinned as a positive control so the three not-null
assertions cannot hold against a function that always answers.
* docs(test): scope the environment guard's docblock to what it actually refuses
Round 4 of the audit ladder found the guard's own documentation claimed more
than the regex delivers. A guard description that reads as coverage while
providing less is worse than none, because it stops the next reader looking.
Two sentences, no behaviour change:
- The scope note now says this refuses two spellings and NOT the class, and
names the three measured escapes (an aliased global, a computed member
access, and a regex literal containing a double slash on the same line as
the read, which the comment stripper truncates). It also records what IS
covered, so the note does not read as an indictment: the spellings a
maintainer would plausibly reach for are caught, and the $app/environment
and $env imports break loudly as a missing module.
- assertEnvironmentIndependent now documents that string literals are
deliberately NOT stripped, so the scanned module may not mention these
tokens in a string either. That fails safe -- red with the guard's own
message, never silently green -- but it is a real constraint, and a
comment is the supported way to write one.
Verified: the three affected suites are 99 passed / 3 files, unchanged.
This commit is contained in:
Zachary Lowden
@@ -26,3 +26,11 @@ packages/civitai-db-schema/src/schema-drift/__tests__/fixtures/catalog-productio
|
||||
packages/civitai-db-schema/src/enums.ts
|
||||
packages/civitai-db-schema/src/models.ts
|
||||
packages/civitai-db-schema/src/kysely/
|
||||
|
||||
# 🔴 THE FORMATTING IS THE FIXTURE. These modules exist so
|
||||
# `src/server/services/__tests__/moderator-restriction-vocabulary.test.ts` can prove its reader sees a
|
||||
# divergence in the moderator app's restriction vocabulary regardless of how that vocabulary is
|
||||
# written — double-quoted entries, a trailing comma, a multi-line array, a comment containing a `]`.
|
||||
# Prettier normalises exactly those differences away, which would silently collapse four distinct
|
||||
# cases into one and leave the suite green while testing a third of what it claims to.
|
||||
src/server/services/__tests__/__fixtures__/moderator-vocabulary/
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
RESTRICTION_TYPE,
|
||||
RESTRICTION_TYPES,
|
||||
RESTRICTION_TYPE_LABELS,
|
||||
RULINGS_WIRED_FOR,
|
||||
unwiredRulingReason,
|
||||
} from '$lib/restriction-types';
|
||||
|
||||
/**
|
||||
* The one predicate three surfaces read: the audit queue's `resolve`/`ban` refusal, the audit queue's
|
||||
* disabled ruling buttons, and the retool User Lookup panel's disabled ruling form. It was open-coded
|
||||
* in the route before #4609's audit; a predicate spelled at N sites is wrong at N−1 of them, and here
|
||||
* the sites disagreeing means a live button whose only possible outcome is a rejected call.
|
||||
*
|
||||
* The refusal that MATTERS is enforced by the main app, inside `resolveUserRestriction` — this list is
|
||||
* the same rule read forward so a form that cannot succeed is never offered. The two are pinned to
|
||||
* each other by `src/server/services/__tests__/restriction-type-seam.test.ts`.
|
||||
*/
|
||||
describe('unwiredRulingReason', () => {
|
||||
it('permits a ruling on every wired-for type', () => {
|
||||
// Non-vacuous: there is at least one, and it is the queue's default.
|
||||
expect(RULINGS_WIRED_FOR.length).toBeGreaterThan(0);
|
||||
expect(RULINGS_WIRED_FOR).toContain(RESTRICTION_TYPE);
|
||||
for (const type of RULINGS_WIRED_FOR) expect(unwiredRulingReason(type)).toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 PINNED BY VALUE, not by "at least one type is refused".
|
||||
*
|
||||
* The test below used to lean on `unwired.length > 0` to stay non-vacuous, and that made it catch
|
||||
* a widened `RULINGS_WIRED_FOR` only BY ACCIDENT — `bot-account` being the sole unwired type is
|
||||
* the only reason wiring it in emptied the list. Add a third filed type and the accident is gone:
|
||||
* `RULINGS_WIRED_FOR` could claim a verdict path for `bot-account` while `unwired` still holds the
|
||||
* third type, so the length check passes and the loop passes and nothing here notices.
|
||||
*
|
||||
* A value pin does not decay that way. It is the mirror of the main app's own
|
||||
* (`expect([...RULINGS_WIRED_FOR]).toEqual(['generation'])` in
|
||||
* `src/server/__tests__/pending-review-mute.test.ts`), and widening this list on either side is
|
||||
* supposed to be a deliberate act with the verdict path parameterised first.
|
||||
*/
|
||||
it('claims a verdict path for generation and for nothing else', () => {
|
||||
expect([...RULINGS_WIRED_FOR]).toEqual(['generation']);
|
||||
});
|
||||
|
||||
it('refuses every filed type that has no verdict path, naming it', () => {
|
||||
const unwired = RESTRICTION_TYPES.filter((t) => !RULINGS_WIRED_FOR.includes(t));
|
||||
// Kept only as a vacuity guard. It is NOT what catches a widened `RULINGS_WIRED_FOR` any more —
|
||||
// the value pin above is, and it does not decay as the vocabulary grows.
|
||||
expect(unwired.length).toBeGreaterThan(0);
|
||||
|
||||
for (const type of unwired) {
|
||||
const reason = unwiredRulingReason(type);
|
||||
// The type is named because a moderator has to be able to tell WHICH queue is review-only, and
|
||||
// the message doubles as the audit route's `fail(400)` body.
|
||||
expect(reason).toContain(`"${type}"`);
|
||||
expect(reason).toContain('NOT resolved');
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses a type nobody has heard of, rather than defaulting it in', () => {
|
||||
// The value reaching this can come off a database row, so it is not confined to the union.
|
||||
for (const type of ['', 'GENERATION', 'generation ', 'nonsense'])
|
||||
expect(unwiredRulingReason(type)).not.toBeNull();
|
||||
});
|
||||
|
||||
it('keeps a label for every filed type, so a refused queue can still be named on screen', () => {
|
||||
expect(Object.keys(RESTRICTION_TYPE_LABELS).sort()).toEqual([...RESTRICTION_TYPES].sort());
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,69 @@
|
||||
/**
|
||||
* The kinds of review that file into the moderator mute queue.
|
||||
*
|
||||
* `UserRestriction.type` is free text in the database with a `[type, status]` index, so a new kind of
|
||||
* review costs no migration — it files into the same queue under a different type. The list is
|
||||
* enumerated here rather than derived from the rows so the queue's filter cannot be driven to an
|
||||
* arbitrary string from the URL, and so a type nobody built a view for cannot render an empty page
|
||||
* that reads as "no work to do".
|
||||
*
|
||||
* 🔴 This file is deliberately OUTSIDE `$lib/server/`. `RestrictionFilters.svelte` renders the type
|
||||
* picker and therefore needs these as VALUES; SvelteKit rejects a value import of `$lib/server/*` from
|
||||
* client-reachable code, and the sibling components only get away with importing from the service
|
||||
* because theirs are `import type` and erase. Keep the list here and re-export it from the service.
|
||||
*
|
||||
* Mirrored for the main app in `src/server/services/user-restriction.service.ts`; the two lists are
|
||||
* pinned to each other by `src/server/services/__tests__/restriction-type-seam.test.ts`, which
|
||||
* imports and executes this module rather than reading it as text.
|
||||
*/
|
||||
export const RESTRICTION_TYPES = ['generation', 'bot-account'] as const;
|
||||
export type RestrictionType = (typeof RESTRICTION_TYPES)[number];
|
||||
|
||||
/** What the queue shows when the URL names no type — the only type that existed before the seam. */
|
||||
export const RESTRICTION_TYPE: RestrictionType = 'generation';
|
||||
|
||||
export const RESTRICTION_TYPE_LABELS: Record<RestrictionType, string> = {
|
||||
generation: 'Generation',
|
||||
'bot-account': 'Bot account',
|
||||
};
|
||||
|
||||
/**
|
||||
* The types a VERDICT can be handed to — narrower than `RESTRICTION_TYPES`, which is what may be
|
||||
* filed and reviewed.
|
||||
*
|
||||
* The refusal itself is enforced by the main app, one level below every ruling surface, in
|
||||
* `resolveUserRestriction` — this app's ruling forms all post through `/api/mod/restriction/resolve`.
|
||||
* This list is the same rule read forward instead of backward: it is what lets a form that cannot
|
||||
* possibly succeed be disabled rather than merely rejected, and what lets the audit queue refuse a
|
||||
* ban BEFORE it bans (that action bans and then rules, so a late refusal would leave the account
|
||||
* banned against a restriction nobody can close).
|
||||
*
|
||||
* 🔴 Kept identical to the main app's `RULINGS_WIRED_FOR` by
|
||||
* `src/server/services/__tests__/restriction-type-seam.test.ts`, which IMPORTS AND EXECUTES this
|
||||
* module and compares the resulting values. The two apps are separate builds with no runtime import
|
||||
* path between them, so a pinned copy is the strongest available form of "one rule, one place" — do
|
||||
* not fork it by hand.
|
||||
*
|
||||
* 🔴 That guard used to parse this file as TEXT, and passed green over a real divergence: a `]` in a
|
||||
* trailing comment truncated its capture. Two preconditions come out of the replacement, and they are
|
||||
* SEPARATE — the second is not implied by the first:
|
||||
*
|
||||
* 1. KEEP THIS MODULE IMPORT-FREE. It has no imports today, and that is the only reason the main
|
||||
* app's Vitest project can load it across the app boundary; adding a `$lib/…` import here breaks
|
||||
* the seam guard loudly rather than silently, but it does break it.
|
||||
* 2. KEEP EVERY VALUE HERE ENVIRONMENT-INDEPENDENT — no `import.meta.env`, no `process.env`, and
|
||||
* nothing derived from them. Neither needs an import statement, so rule 1 does not cover this.
|
||||
* The guard EXECUTES this module inside the main app's test process: a list that branches on the
|
||||
* environment is read under Vitest and never under this app's production build, so the two apps
|
||||
* can ship different lists with every guard on both sides green. Assembling a value from
|
||||
* constants declared in this file is fine — that is the same value everywhere. Reading one from
|
||||
* the environment is not, and is refused by name.
|
||||
*/
|
||||
export const RULINGS_WIRED_FOR: readonly RestrictionType[] = ['generation'];
|
||||
|
||||
/** Why a ruling may not be handed to a row of this type, or `null` when it may. */
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
return (RULINGS_WIRED_FOR as readonly string[]).includes(type)
|
||||
? null
|
||||
: `Rulings are not yet available for "${type}" restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.`;
|
||||
}
|
||||
@@ -0,0 +1,130 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/**
|
||||
* `UserRestriction.type` is what separates one review queue from another. The rows carry no other
|
||||
* marker — same table, same status vocabulary, same shape — so if the predicate is dropped, weakened,
|
||||
* or bound to the wrong value, one queue simply renders another's cases and a moderator rules on them
|
||||
* under the wrong assumptions.
|
||||
*
|
||||
* 🔴 Asserted against the COMPILED SQL **and its bound parameters**. The text alone cannot see the bug
|
||||
* this file exists for: `where('ur.type','=',x)` emits `"ur"."type" = $1` whatever `x` is, so a version
|
||||
* that ignores its argument and always filters `generation` produces byte-identical SQL. The parameter
|
||||
* is the only place the difference is visible.
|
||||
*
|
||||
* The count query is checked alongside the row query on purpose — they are separately compiled off a
|
||||
* shared builder, and a filter that reaches one but not the other gives a pager whose total counts
|
||||
* every type's rows.
|
||||
*/
|
||||
|
||||
const captured = vi.hoisted(() => [] as string[]);
|
||||
const capturedParams = vi.hoisted(() => [] as unknown[][]);
|
||||
|
||||
// Built inside the factory, not in `vi.hoisted`: hoisted blocks run before this file's own imports, so
|
||||
// constructing the client there reads it before initialisation.
|
||||
vi.mock('$lib/server/db', async () => {
|
||||
const { capturingDb } = await import('../../../test/capture-sql');
|
||||
const db = capturingDb(captured, [], capturedParams);
|
||||
return { dbRead: db, dbWrite: db };
|
||||
});
|
||||
|
||||
const { getGenerationRestrictions, RESTRICTION_TYPE } = await import('../user-restriction.service');
|
||||
|
||||
type Compiled = { sql: string; params: unknown[] };
|
||||
|
||||
const compile = async (
|
||||
query: Parameters<typeof getGenerationRestrictions>[0]
|
||||
): Promise<Compiled[]> => {
|
||||
captured.length = 0;
|
||||
capturedParams.length = 0;
|
||||
await getGenerationRestrictions(query);
|
||||
// 🔴 Assert the COUNT, not just the contents. A chain that stops early leaves `captured` short while
|
||||
// every assertion over what IS in it still passes — on a query that was never built.
|
||||
expect(captured).toHaveLength(2);
|
||||
return captured.map((sql, i) => ({ sql, params: capturedParams[i] }));
|
||||
};
|
||||
|
||||
const base = { page: 1, limit: 20 } as const;
|
||||
|
||||
/** The `ur.type` predicate's bound value, or `undefined` if the query emitted no such predicate. */
|
||||
const boundType = ({ sql, params }: Compiled): unknown => {
|
||||
const match = /"ur"\."type" = \$(\d+)/.exec(sql);
|
||||
return match ? params[Number(match[1]) - 1] : undefined;
|
||||
};
|
||||
|
||||
describe('getGenerationRestrictions — type scoping', () => {
|
||||
it('filters on the generation type when the caller names none', async () => {
|
||||
const compiled = await compile({ ...base });
|
||||
|
||||
// Both statements, not just the first: the second is the pager's total.
|
||||
for (const c of compiled) expect(boundType(c)).toBe('generation');
|
||||
expect(RESTRICTION_TYPE).toBe('generation');
|
||||
});
|
||||
|
||||
it('filters on the type it was given', async () => {
|
||||
const compiled = await compile({ ...base, type: 'bot-account' });
|
||||
|
||||
for (const c of compiled) expect(boundType(c)).toBe('bot-account');
|
||||
});
|
||||
|
||||
// 🔴 The leak, stated as a property rather than as one example: whatever type is asked for, no other
|
||||
// type's rows can satisfy the query. A predicate bound to the requested value is what guarantees it.
|
||||
it.each(['generation', 'bot-account'] as const)(
|
||||
'binds %s and no other type, in both the list and the count',
|
||||
async (type) => {
|
||||
const compiled = await compile({ ...base, type });
|
||||
|
||||
for (const c of compiled) {
|
||||
expect(boundType(c)).toBe(type);
|
||||
// Exactly one type PREDICATE — a second, differently-bound one would AND/OR in another queue.
|
||||
// Matched with the `= $n` so this counts predicates and not the `ur.type` in the SELECT list.
|
||||
expect(c.sql.match(/"ur"\."type" = \$\d+/g)).toHaveLength(1);
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
it('keeps the type predicate when other filters are also applied', async () => {
|
||||
// The type predicate is unconditional while the rest are `$if`s. A refactor that folded it in with
|
||||
// them is exactly how it would go missing, and only a query carrying both shapes can see that.
|
||||
const compiled = await compile({
|
||||
...base,
|
||||
type: 'bot-account',
|
||||
status: 'Pending',
|
||||
username: 'someone',
|
||||
});
|
||||
|
||||
for (const c of compiled) {
|
||||
expect(boundType(c)).toBe('bot-account');
|
||||
expect(c.sql).toMatch(/"ur"\."status" = \$/);
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* The one caller allowed past the filter: a lookup by primary key. A form posts to `?/resolve`, which
|
||||
* replaces the query string, so an action never learns which queue the moderator was in — filtering a
|
||||
* by-id lookup by the DEFAULT type would 404 every row outside it. A primary key cannot be made more
|
||||
* correct by a type predicate, so the predicate is dropped rather than guessed.
|
||||
*/
|
||||
it("drops the predicate only for the explicit 'any'", async () => {
|
||||
const compiled = await compile({ ...base, type: 'any', restrictionId: 7 });
|
||||
|
||||
for (const c of compiled) {
|
||||
expect(boundType(c)).toBeUndefined();
|
||||
// The PREDICATE is gone. `ur.type` still appears in the row query's SELECT list, which is what
|
||||
// lets the caller read back the type it did not filter on.
|
||||
expect(c.sql).not.toMatch(/"ur"\."type" = \$/);
|
||||
expect(c.sql).toMatch(/"ur"\."id" = \$/);
|
||||
}
|
||||
});
|
||||
|
||||
it('selects the row type, so a caller can tell what it got back', async () => {
|
||||
// `restrictionById` reads it to decide whether a ruling is wired for the row; without it in the
|
||||
// SELECT that check silently compares against `undefined`.
|
||||
const [list] = await compile({ ...base });
|
||||
|
||||
// 🔴 Sliced to the SELECT list first. Asserted against the whole statement, this passes on the
|
||||
// `"ur"."type" = $1` in the WHERE — which every pre-change version also emitted — so the guard
|
||||
// would report coverage of a column that is not being selected at all.
|
||||
const selectList = list.sql.slice(0, list.sql.indexOf(' from "UserRestriction"'));
|
||||
expect(selectList).toContain('"ur"."type"');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,124 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/**
|
||||
* Which UserRestriction row the User Lookup header and its ruling form speak for.
|
||||
*
|
||||
* 🔴 The panel shows ONE row, and until #4609 it picked "newest of any type" — `ORDER BY ur.id DESC
|
||||
* LIMIT 1`, no type predicate. That was sound only while a user could hold at most one open row.
|
||||
* Restrictions now dedupe PER TYPE, so two open cases can coexist, and the old ordering had a silent
|
||||
* failure mode: a Pending generation case sitting behind a LATER Upheld bot-account row rendered as
|
||||
* *no open restriction at all* — the account stays muted, the ruling form is never drawn, and nobody
|
||||
* looking at the account can see there is an open case.
|
||||
*
|
||||
* Asserted against the COMPILED SQL. There is no database in this tier, and the ordering is the whole
|
||||
* behaviour — it decides which of several rows the moderator is shown, and it typechecks and lints
|
||||
* identically either way. Same instrument the sibling `restriction-type-filter.test.ts` uses, for the
|
||||
* same reason.
|
||||
*/
|
||||
|
||||
const captured = vi.hoisted(() => [] as string[]);
|
||||
|
||||
// Built inside the factory, not in `vi.hoisted`: hoisted blocks run before this file's own imports.
|
||||
vi.mock('$lib/server/db', async () => {
|
||||
const { capturingDb } = await import('../../../test/capture-sql');
|
||||
// One canned row, so `executeTakeFirst` resolves and the chain does not stop early.
|
||||
const db = capturingDb(captured, [{ id: 1 }]);
|
||||
return { dbRead: db, dbWrite: db };
|
||||
});
|
||||
|
||||
// The identity query is the only thing under test here; its module's other exports reach a second
|
||||
// database and an HTTP service, and none of them is on this path.
|
||||
const { getIdentity } = await import('../user-lookup.service');
|
||||
|
||||
const identitySql = async (): Promise<string> => {
|
||||
captured.length = 0;
|
||||
await getIdentity(42);
|
||||
// 🔴 Count first. A chain that stopped early would leave `captured` empty while every assertion over
|
||||
// its contents passed vacuously.
|
||||
expect(captured).toHaveLength(1);
|
||||
return captured[0].replace(/\s+/g, ' ');
|
||||
};
|
||||
|
||||
/**
|
||||
* The three correlated subqueries that read the account's restriction, sliced from the head of each
|
||||
* `FROM "UserRestriction" ur` to its `LIMIT 1`.
|
||||
*/
|
||||
const restrictionSubqueries = (sql: string): string[] =>
|
||||
sql
|
||||
.split('FROM "UserRestriction" ur')
|
||||
.slice(1)
|
||||
.map((chunk) => {
|
||||
const end = chunk.indexOf('LIMIT 1');
|
||||
expect(end).toBeGreaterThan(-1);
|
||||
return chunk.slice(0, end).trim();
|
||||
});
|
||||
|
||||
describe('user lookup — which restriction the panel speaks for', () => {
|
||||
it('reads three restriction columns, which is what the panel needs to render a ruling form', async () => {
|
||||
const sql = await identitySql();
|
||||
|
||||
// Positive control on the slicer, and the thing that makes every assertion below non-vacuous: a
|
||||
// rename or a fourth column shows up here rather than silently reducing the loops to no-ops.
|
||||
expect(restrictionSubqueries(sql)).toHaveLength(3);
|
||||
for (const alias of ['restrictionStatus', 'restrictionType', 'restrictionId'])
|
||||
expect(sql).toContain(`as "${alias}"`);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The regression. A Pending row outranks a merely newer one, so an open case cannot be hidden
|
||||
* behind a later resolved one of another type.
|
||||
*/
|
||||
it('prefers a Pending row over a newer one, in every restriction column', async () => {
|
||||
const sql = await identitySql();
|
||||
|
||||
for (const sub of restrictionSubqueries(sql))
|
||||
expect(sub).toContain(`ORDER BY (ur.status = 'Pending') DESC NULLS LAST, ur.id DESC`);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The null placement, asserted on its own so it cannot be dropped by someone tidying the
|
||||
* ordering above back to a bare `DESC`.
|
||||
*
|
||||
* Postgres defaults `DESC` to `NULLS FIRST`. A NULL `(ur.status = 'Pending')` would therefore sort
|
||||
* ABOVE a genuinely Pending row and hide the open case — exactly the failure the preference exists
|
||||
* to prevent, arriving through the column's nullability rather than through the ordering. The
|
||||
* column is a NOT NULL enum today, so this is an unstated precondition being made explicit rather
|
||||
* than a live defect; spelled out, the ordering no longer depends on it.
|
||||
*/
|
||||
it('places nulls last, so the ordering does not depend on status being NOT NULL', async () => {
|
||||
const sql = await identitySql();
|
||||
const subs = restrictionSubqueries(sql);
|
||||
|
||||
// Non-vacuous: three subqueries exist to check. (The slicer's own control lives in the first
|
||||
// test; this repeats the count because an empty list would make the loop below pass.)
|
||||
expect(subs).toHaveLength(3);
|
||||
for (const sub of subs) {
|
||||
expect(sub).toContain(`'Pending') DESC NULLS LAST`);
|
||||
// And not a bare `DESC` on that expression, which is what the default null placement is.
|
||||
expect(sub).not.toMatch(/'Pending'\)\s+DESC\s*,/);
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* The tiebreak has to be TOTAL, or the three subqueries are free to resolve to different rows and
|
||||
* the panel renders one row's status against another's id — a ruling form posting an id that does
|
||||
* not belong to the case it is describing.
|
||||
*/
|
||||
it('breaks the tie on a unique column, so the three columns name one row', async () => {
|
||||
const sql = await identitySql();
|
||||
const subs = restrictionSubqueries(sql);
|
||||
|
||||
for (const sub of subs) expect(sub).toContain('ur.id DESC');
|
||||
// Identical apart from the column each selects — which is the only reason they agree. (An
|
||||
// INVARIANT GUARD: true before this change too. Kept because the fix replaced three hand-written
|
||||
// copies with one helper, and this is what stops them being hand-written again.)
|
||||
const withoutSelectList = subs.map((s) => s.slice(s.indexOf('WHERE')));
|
||||
expect(new Set(withoutSelectList).size).toBe(1);
|
||||
});
|
||||
|
||||
it('scopes each subquery to the account being looked up', async () => {
|
||||
const sql = await identitySql();
|
||||
|
||||
for (const sub of restrictionSubqueries(sql)) expect(sub).toContain(`ur."userId" = u.id`);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,49 @@
|
||||
/**
|
||||
* The refusal a main-app endpoint wrote for the operator, read out of its JSON error body.
|
||||
*
|
||||
* 🔴 TWO ENVELOPE SHAPES REACH THIS APP, and reading only one of them silently destroys the reason.
|
||||
* `defineModeratorEndpoint` hands a throw to the main app's `handleEndpointError`
|
||||
* (`src/server/utils/endpoint-helpers.ts`), and that helper emits:
|
||||
*
|
||||
* - `{ error, message, code }` — every `restErrorBody(...)` path, which is what the 5xx branch and
|
||||
* the genericized-4xx branch use;
|
||||
* - `{ message }` alone — the 4xx/503 PASS-THROUGH branch, i.e. exactly the statuses that carry a
|
||||
* refusal somebody wrote FOR a human (`throwBadRequestError`, `throwNotFoundError`, …).
|
||||
*
|
||||
* Reading `error` only therefore came back `null` for the entire second group, and the caller fell
|
||||
* back to `"<label> returned <status>."` — the status with the reason stripped off it. Measured on
|
||||
* #4609: a moderator ruling on an already-ruled restriction saw "Restriction ruling returned 400."
|
||||
* That is the same class of loss as the opaque 500 the endpoint change was made to remove, one layer
|
||||
* further out.
|
||||
*
|
||||
* The fix is here rather than in `handleEndpointError` deliberately. That helper is the shared
|
||||
* chokepoint for 36 REST route files, its 4xx and 503 pass-through bodies are pinned
|
||||
* `toStrictEqual({ message })` by `endpoint-helpers-error-envelope.test.ts` (the 503 case as an
|
||||
* explicit, documented carve-out), and `restErrorBody` needs a `RestErrorCode` that is not derivable
|
||||
* at that point without a new status→code map — which would then have to be reconciled with the
|
||||
* closed key ledger `rest-error-envelope-ledger.test.ts` enforces. Widening the reader costs one
|
||||
* expression and makes every endpoint's 4xx legible here; widening the emitter changes the wire
|
||||
* format of 36 routes.
|
||||
*
|
||||
* 🔴 This module is deliberately IMPORT-FREE, for the same reason
|
||||
* `apps/moderator/src/lib/restriction-types.ts` is: the main app's Vitest project loads it across the
|
||||
* app boundary by filesystem path, so that the seam between the emitter and this reader can be
|
||||
* driven end-to-end in ONE process
|
||||
* (`src/server/__tests__/pending-review-mute.test.ts`). Two suites that each mock the other side
|
||||
* would both pass over exactly the divergence above. Keep it import-free.
|
||||
*/
|
||||
export function restErrorReason(body: unknown, status: number): string | null {
|
||||
const b = (body ?? null) as Record<string, unknown> | null;
|
||||
const reason =
|
||||
typeof b?.error === 'string'
|
||||
? b.error
|
||||
: // The 4xx/503 pass-through shape. Second, not first: where both keys exist they are the same
|
||||
// string (`restErrorBody` defaults `error` to `message`), so the order is only about which
|
||||
// one wins if they ever diverge, and `error` is the older contract.
|
||||
typeof b?.message === 'string'
|
||||
? b.message
|
||||
: null;
|
||||
if (!reason) return null;
|
||||
const retry = b?.retryAfterSeconds;
|
||||
return status === 429 && typeof retry === 'number' ? `${reason} — retry in ${retry}s.` : reason;
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import { bustUserCosmeticCaches } from './cache';
|
||||
import { getModeratorDb } from './moderator-db';
|
||||
import { recordModActivity } from './mod-activity';
|
||||
import { recordUserActivity } from './user-activity';
|
||||
import { restErrorReason } from './rest-error-reason';
|
||||
import { invalidateUserSessions } from './sessions';
|
||||
import { PROFILE_FIELD_KEYS, type ProfileField } from '$lib/enforcement';
|
||||
|
||||
@@ -57,13 +58,13 @@ async function callMainApp(
|
||||
export type JsonResult = { ok: true; body: Record<string, unknown> } | { ok: false; error: string };
|
||||
|
||||
/** The refusal an endpoint wrote for the operator. A rate limit also carries how long is left, which
|
||||
* is the difference between "try later" and a moderator retrying immediately. */
|
||||
* is the difference between "try later" and a moderator retrying immediately.
|
||||
*
|
||||
* The body-shape rule lives in `./rest-error-reason` because the main app emits TWO envelopes and
|
||||
* reading only one drops the reason — see that file. It is kept import-free so the main app's suite
|
||||
* can drive the real emitter into this real reader in one process. */
|
||||
async function readError(res: Response): Promise<string | null> {
|
||||
const body = (await res.json().catch(() => null)) as Record<string, unknown> | null;
|
||||
const error = typeof body?.error === 'string' ? body.error : null;
|
||||
if (!error) return null;
|
||||
const retry = body?.retryAfterSeconds;
|
||||
return res.status === 429 && typeof retry === 'number' ? `${error} — retry in ${retry}s.` : error;
|
||||
return restErrorReason(await res.json().catch(() => null), res.status);
|
||||
}
|
||||
|
||||
/** The one JSON poster. Two auth schemes because the endpoint families disagree, not because the
|
||||
|
||||
@@ -381,7 +381,45 @@ export async function getLeaderboardRanks(userId: number): Promise<LeaderboardRa
|
||||
);
|
||||
}
|
||||
|
||||
async function getIdentity(userId: number): Promise<UserIdentity | null> {
|
||||
/**
|
||||
* One field of the ONE restriction row this panel speaks for.
|
||||
*
|
||||
* 🔴 `ORDER BY (ur.status = 'Pending') DESC` first, `ur.id DESC` second — a PENDING case outranks a
|
||||
* merely newer one. The old ordering was newest-of-any-type, which was sound only while a user could
|
||||
* hold at most one open row. Restrictions now dedupe PER TYPE, so two open cases can coexist: a
|
||||
* Pending generation case sitting behind a later Upheld bot-account row rendered as *no open
|
||||
* restriction at all*, leaving the account muted, the ruling form unrendered, and nobody able to see
|
||||
* the open case. Preferring Pending is what makes that impossible in either direction.
|
||||
*
|
||||
* The panel shows ONE row on purpose — it is a header chip plus a single ruling form, and the audit
|
||||
* queue is where a list of cases belongs. So the one it shows is the one that can still be acted on;
|
||||
* among several Pending rows it shows the newest, and a resolved row is only ever shown when there is
|
||||
* no open one.
|
||||
*
|
||||
* Written once and called three times rather than spelled out per column: the three subqueries must
|
||||
* name the SAME row, and three copies of an ordering rule is three places for it to stop agreeing.
|
||||
* `ur.id DESC` makes the order total, so all three resolve to that same row deterministically.
|
||||
* `expr` is a literal from this module, never caller input.
|
||||
*
|
||||
* 🔴 `DESC NULLS LAST`, not bare `DESC`. Postgres sorts NULLs FIRST under `DESC`, so `(ur.status =
|
||||
* 'Pending')` evaluating to NULL would outrank an actual Pending row and put the panel back in the
|
||||
* state this ordering exists to prevent — a real open case hidden behind another row. `ur.status` is
|
||||
* a NOT NULL enum today, which makes bare `DESC` correct by a precondition nothing here states or
|
||||
* checks; spelling the null placement makes the ordering independent of the column's nullability
|
||||
* instead of quietly depending on it.
|
||||
*/
|
||||
function restrictionField<T>(expr: string) {
|
||||
return sql<T>`(
|
||||
SELECT ${sql.raw(expr)} FROM "UserRestriction" ur
|
||||
WHERE ur."userId" = u.id
|
||||
ORDER BY (ur.status = 'Pending') DESC NULLS LAST, ur.id DESC
|
||||
LIMIT 1
|
||||
)`;
|
||||
}
|
||||
|
||||
// Exported for the SQL-shape test — `getUserLookup` fans out over two databases, so reaching this
|
||||
// query through it would be a test about the mocks rather than about the ordering above.
|
||||
export async function getIdentity(userId: number): Promise<UserIdentity | null> {
|
||||
const row = await dbRead
|
||||
.selectFrom('User as u')
|
||||
.select([
|
||||
@@ -418,18 +456,9 @@ async function getIdentity(userId: number): Promise<UserIdentity | null> {
|
||||
sql<number>`(SELECT COUNT(*)::int FROM "CsamReport" cr WHERE cr."userId" = u.id)`.as(
|
||||
'csamReportCount'
|
||||
),
|
||||
sql<string | null>`(
|
||||
SELECT ur.status::text FROM "UserRestriction" ur
|
||||
WHERE ur."userId" = u.id ORDER BY ur.id DESC LIMIT 1
|
||||
)`.as('restrictionStatus'),
|
||||
sql<string | null>`(
|
||||
SELECT ur.type FROM "UserRestriction" ur
|
||||
WHERE ur."userId" = u.id ORDER BY ur.id DESC LIMIT 1
|
||||
)`.as('restrictionType'),
|
||||
sql<number | null>`(
|
||||
SELECT ur.id FROM "UserRestriction" ur
|
||||
WHERE ur."userId" = u.id ORDER BY ur.id DESC LIMIT 1
|
||||
)`.as('restrictionId'),
|
||||
restrictionField<string | null>('ur.status::text').as('restrictionStatus'),
|
||||
restrictionField<string | null>('ur.type').as('restrictionType'),
|
||||
restrictionField<number | null>('ur.id').as('restrictionId'),
|
||||
// The ticket asked for open reports against the account "very clearly at the top". A report
|
||||
// nobody has ruled on changes what every other panel means, and it was reachable only by
|
||||
// navigating to the Reports section and reading a list.
|
||||
|
||||
@@ -3,7 +3,18 @@ import { REDIS_SYS_KEYS } from '@civitai/redis';
|
||||
import { dbRead } from './db';
|
||||
import { getSysRedis } from './redis';
|
||||
|
||||
export const RESTRICTION_TYPE = 'generation';
|
||||
import { RESTRICTION_TYPE, type RestrictionType } from '$lib/restriction-types';
|
||||
|
||||
// Re-exported so server-side callers have one import site for the queue's vocabulary. The definitions
|
||||
// live in a client-safe module because the filter component needs them as values — see the note there.
|
||||
export {
|
||||
RESTRICTION_TYPE,
|
||||
RESTRICTION_TYPES,
|
||||
RESTRICTION_TYPE_LABELS,
|
||||
RULINGS_WIRED_FOR,
|
||||
unwiredRulingReason,
|
||||
type RestrictionType,
|
||||
} from '$lib/restriction-types';
|
||||
|
||||
/** One prohibited request recorded against a restriction. Every field is optional: the shape is
|
||||
* written by two producers (the live audit and the ClickHouse backfill) and older rows predate both. */
|
||||
@@ -27,6 +38,8 @@ export type RestrictionRow = {
|
||||
id: number;
|
||||
userId: number;
|
||||
username: string | null;
|
||||
/** Read back rather than assumed: a by-id lookup is type-agnostic, so the caller cannot infer it. */
|
||||
type: string;
|
||||
status: UserRestrictionStatus;
|
||||
createdAt: Date;
|
||||
resolvedAt: Date | null;
|
||||
@@ -39,6 +52,14 @@ export type RestrictionRow = {
|
||||
export type RestrictionQuery = {
|
||||
page: number;
|
||||
limit: number;
|
||||
/**
|
||||
* Omitted means `generation`, so every pre-seam caller keeps the queue it had.
|
||||
*
|
||||
* `'any'` drops the filter, and exists for the ONE caller that addresses a row by its primary key —
|
||||
* where a type predicate cannot make the answer more correct, only turn a real row into a 404. It is
|
||||
* deliberately absent from the page's query schema, so no URL can put the list view into it.
|
||||
*/
|
||||
type?: RestrictionType | 'any';
|
||||
status?: UserRestrictionStatus;
|
||||
username?: string;
|
||||
userId?: number;
|
||||
@@ -69,12 +90,17 @@ export async function getGenerationRestrictions(query: RestrictionQuery): Promis
|
||||
items: RestrictionRow[];
|
||||
totalCount: number;
|
||||
}> {
|
||||
const { page, limit, status, username, userId, restrictionId } = query;
|
||||
const { page, limit, type = RESTRICTION_TYPE, status, username, userId, restrictionId } = query;
|
||||
|
||||
// 🔴 The type predicate is what separates one review queue from another, and it is applied for every
|
||||
// value of `type` except the explicit `'any'`. It is written as a `$if` on a NEGATIVE so that
|
||||
// omitting `type` still filters — folding it in with the optional predicates below on a truthiness
|
||||
// test would make "no type given" mean "every type", i.e. silently render one queue's rows in
|
||||
// another's. The count query is built off this same `base`, so the total cannot drift from the list.
|
||||
const base = dbRead
|
||||
.selectFrom('UserRestriction as ur')
|
||||
.innerJoin('User as u', 'u.id', 'ur.userId')
|
||||
.where('ur.type', '=', RESTRICTION_TYPE)
|
||||
.$if(type !== 'any', (qb) => qb.where('ur.type', '=', type))
|
||||
.where('u.deletedAt', 'is', null)
|
||||
.$if(!!status, (qb) => qb.where('ur.status', '=', status!))
|
||||
.$if(!!userId, (qb) => qb.where('ur.userId', '=', userId!))
|
||||
@@ -87,6 +113,7 @@ export async function getGenerationRestrictions(query: RestrictionQuery): Promis
|
||||
'ur.id',
|
||||
'ur.userId',
|
||||
'u.username',
|
||||
'ur.type',
|
||||
'ur.status',
|
||||
'ur.triggers',
|
||||
'ur.createdAt',
|
||||
|
||||
@@ -2,13 +2,15 @@ import { fail } from '@sveltejs/kit';
|
||||
import { z } from 'zod';
|
||||
import type { Actions, PageServerLoad } from './$types';
|
||||
import { parseForm, parseQuery } from '$lib/server/query';
|
||||
import { dbWrite } from '$lib/server/db';
|
||||
import { requiresGrant } from '$lib/server/access';
|
||||
import { banFieldsSchema, banRemovalArgs, rejectUnexplainedOther } from '$lib/server/ban-input';
|
||||
import { banConfirmed, resolveRestriction, setBanned } from '$lib/server/user-actions.service';
|
||||
import {
|
||||
getGenerationRestrictions,
|
||||
saveSuspiciousMatches,
|
||||
RESTRICTION_TYPE,
|
||||
RESTRICTION_TYPES,
|
||||
unwiredRulingReason,
|
||||
type RestrictionRow,
|
||||
} from '$lib/server/user-restriction.service';
|
||||
|
||||
@@ -16,7 +18,13 @@ const PAGE_SIZE = 20;
|
||||
|
||||
// Defaults to Pending, as the main app's page did. Without it the queue opens with already-ruled rows
|
||||
// interleaved at the top, and `advance()` has nothing to advance past.
|
||||
//
|
||||
// `type` has no "any" member, unlike `status`. Mixing two kinds of review into one list would put a
|
||||
// moderator one keystroke from ruling on a case with the wrong queue's assumptions in mind, and the
|
||||
// ruling copy differs per type — so the queues stay disjoint and `.catch()` sends an unknown or absent
|
||||
// value back to the type this page has always shown.
|
||||
const querySchema = z.object({
|
||||
type: z.enum(RESTRICTION_TYPES).catch(RESTRICTION_TYPE),
|
||||
status: z.enum(['Pending', 'Upheld', 'Overturned', 'any']).catch('Pending'),
|
||||
q: z.string().trim().max(100).catch(''),
|
||||
page: z.coerce.number().int().min(1).max(500).catch(1),
|
||||
@@ -24,7 +32,7 @@ const querySchema = z.object({
|
||||
});
|
||||
|
||||
export const load: PageServerLoad = async ({ url }) => {
|
||||
const { status, q, page, selected } = parseQuery(url, querySchema);
|
||||
const { type, status, q, page, selected } = parseQuery(url, querySchema);
|
||||
|
||||
// A bare number is a user id, not a username: usernames are free text and an account named "12345"
|
||||
// would otherwise be the only way to reach user 12345.
|
||||
@@ -34,6 +42,7 @@ export const load: PageServerLoad = async ({ url }) => {
|
||||
const { items, totalCount } = await getGenerationRestrictions({
|
||||
page,
|
||||
limit: PAGE_SIZE,
|
||||
type,
|
||||
status: status === 'any' ? undefined : status,
|
||||
username: !isUserId && q ? q : undefined,
|
||||
userId: isUserId ? asId : undefined,
|
||||
@@ -47,17 +56,62 @@ export const load: PageServerLoad = async ({ url }) => {
|
||||
totalCount,
|
||||
page,
|
||||
pageCount: Math.max(1, Math.ceil(totalCount / PAGE_SIZE)),
|
||||
type,
|
||||
status,
|
||||
q,
|
||||
wide: true,
|
||||
};
|
||||
};
|
||||
|
||||
// `type: 'any'` on purpose. A form posts to `?/resolve`, which REPLACES the query string — so an action
|
||||
// never sees the `type` the moderator was looking at, and defaulting the lookup would 404 every row
|
||||
// outside the default queue. The id is a primary key, so dropping the predicate cannot widen the
|
||||
// result; what it changes is which types an action can be handed.
|
||||
//
|
||||
// Each action decides that for itself, and they do NOT all decide the same way — so this helper deliberately
|
||||
// makes no such decision. `resolve` and `ban` call `unwiredRuling` because they hand the row to a verdict
|
||||
// path that is still generation-shaped. `flagSuspicious` does not, and should not: it copies selected
|
||||
// triggers into the shared suspicious-match list, writes nothing to the account, and tells the user
|
||||
// nothing. A prompt worth flagging is worth flagging whatever queue it was raised in.
|
||||
async function restrictionById(id: number): Promise<RestrictionRow | null> {
|
||||
const { items } = await getGenerationRestrictions({ page: 1, limit: 1, restrictionId: id });
|
||||
const { items } = await getGenerationRestrictions({
|
||||
page: 1,
|
||||
limit: 1,
|
||||
type: 'any',
|
||||
restrictionId: id,
|
||||
});
|
||||
return items[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* 🔴 Verdicts are still generation-shaped, so only generation rows may be ruled on.
|
||||
*
|
||||
* The main app's `resolveUserRestriction` — the single write path for a verdict — hardcodes the
|
||||
* `generation-restriction-upheld` / `-overturned` notification types, a `moderator:generationRestriction*`
|
||||
* update source, and a `restriction-upheld` / `-overturned` email, and on an overturn it resets the
|
||||
* *prompt* violation counter. Ruling on a non-generation row through it would tell the user their
|
||||
* generation access was restored over something unrelated to generation.
|
||||
*
|
||||
* This is a refusal rather than a hidden button because the check has to hold against a posted id, not
|
||||
* just against what the page chose to render. Lifting it means parameterising that verdict path first.
|
||||
* (The buttons are disabled as well now — see `RestrictionDetail.svelte`. That is an addition to this
|
||||
* check, never a substitute for it.)
|
||||
*
|
||||
* 🔴 KEPT even though the refusal is now enforced by `resolveUserRestriction` itself — which is what
|
||||
* closes the surfaces this check could never reach: the retool User Lookup panel, the tRPC router and
|
||||
* the REST endpoint were all unguarded while it lived only here. This is not defence in depth; it is
|
||||
* ORDERING. The `ban` action bans and THEN rules, so a refusal arriving inside the verdict call would
|
||||
* leave the account banned against a restriction nobody can close — the stranded Pending row that
|
||||
* handler exists to avoid. It also renders as an inline message rather than a failed API call.
|
||||
*
|
||||
* The predicate is IMPORTED, never re-spelled: `unwiredRulingReason` is one function in
|
||||
* `$lib/restriction-types`, pinned to the main app's `RULINGS_WIRED_FOR` by the seam test. A second
|
||||
* spelling here is exactly how the two would drift.
|
||||
*/
|
||||
function unwiredRuling(row: RestrictionRow): string | null {
|
||||
return unwiredRulingReason(row.type);
|
||||
}
|
||||
|
||||
export const actions: Actions = {
|
||||
resolve: async ({ request, locals }) => {
|
||||
const input = parseForm(
|
||||
@@ -73,6 +127,8 @@ export const actions: Actions = {
|
||||
// posted one lets the audit trail record an account that was never acted on.
|
||||
const row = await restrictionById(input.userRestrictionId);
|
||||
if (!row) return fail(404, { error: 'Restriction not found.' });
|
||||
const unwired = unwiredRuling(row);
|
||||
if (unwired) return fail(400, { error: unwired });
|
||||
|
||||
const result = await resolveRestriction({
|
||||
userRestrictionId: input.userRestrictionId,
|
||||
@@ -97,6 +153,11 @@ export const actions: Actions = {
|
||||
// The account banned is the restriction's owner, not whoever the form named.
|
||||
const row = await restrictionById(input.userRestrictionId);
|
||||
if (!row) return fail(404, { error: 'Restriction not found.' });
|
||||
// Checked BEFORE the ban, not just before the resolve: this action bans and then rules, and a ban
|
||||
// that landed against a restriction that cannot be resolved leaves exactly the stranded Pending row
|
||||
// the `ban` handler exists to avoid.
|
||||
const unwired = unwiredRuling(row);
|
||||
if (unwired) return fail(400, { error: unwired });
|
||||
|
||||
const banned = await setBanned({
|
||||
userId: row.userId,
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import RestrictionDetail from './RestrictionDetail.svelte';
|
||||
import StatusBadge from './StatusBadge.svelte';
|
||||
import Pager from '$lib/components/Pager.svelte';
|
||||
import { RESTRICTION_TYPE, RESTRICTION_TYPE_LABELS } from '$lib/restriction-types';
|
||||
import type { PageData } from './$types';
|
||||
|
||||
let { data }: { data: PageData } = $props();
|
||||
@@ -35,15 +36,27 @@
|
||||
|
||||
<header class="page-header">
|
||||
<h1>Generator Restrictions</h1>
|
||||
<p>Generation restrictions raised by the prompt-auditing system, and the rulings on them.</p>
|
||||
{#if data.type === RESTRICTION_TYPE}
|
||||
<p>Generation restrictions raised by the prompt-auditing system, and the rulings on them.</p>
|
||||
{:else}
|
||||
<!-- Named rather than described: the verdict path still sends generation-specific notices, so the
|
||||
resolve and ban actions refuse these rows server-side. Saying so here is what stops a
|
||||
moderator reading that refusal as a bug. -->
|
||||
<p>
|
||||
{RESTRICTION_TYPE_LABELS[data.type]} restrictions. Review only — rulings are not yet wired for this
|
||||
type.
|
||||
</p>
|
||||
{/if}
|
||||
</header>
|
||||
|
||||
<RestrictionFilters q={data.q} status={data.status} />
|
||||
<RestrictionFilters q={data.q} status={data.status} type={data.type} />
|
||||
|
||||
<div class="flex items-start gap-6">
|
||||
<div class="flex w-104 shrink-0 flex-col">
|
||||
{#if data.items.length === 0}
|
||||
<p class="text-sm text-dark-2">No generation restrictions match these filters.</p>
|
||||
<p class="text-sm text-dark-2">
|
||||
No {RESTRICTION_TYPE_LABELS[data.type].toLowerCase()} restrictions match these filters.
|
||||
</p>
|
||||
{:else}
|
||||
<ul class="max-h-[70vh] overflow-auto rounded-xl border border-dark-4">
|
||||
{#each data.items as item (item.id)}
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
import { LINK_CLASS, dateTime } from '$lib/format';
|
||||
import { userLookupUrl } from '$lib/entity-url';
|
||||
import type { RestrictionRow } from '$lib/server/user-restriction.service';
|
||||
import { unwiredRulingReason } from '$lib/restriction-types';
|
||||
import UserWorkflowsPanel from '$lib/components/UserWorkflowsPanel.svelte';
|
||||
import TriggerCard from './TriggerCard.svelte';
|
||||
import StatusBadge from './StatusBadge.svelte';
|
||||
@@ -34,6 +35,13 @@
|
||||
const selected = new SvelteSet<string>();
|
||||
const toggle = (key: string) => (selected.has(key) ? selected.delete(key) : selected.add(key));
|
||||
|
||||
// The SERVER refuses these rows too, and that refusal is the guard — it holds against a posted id,
|
||||
// which nothing rendered here can. This only stops the moderator being INVITED to click a button
|
||||
// that can never do anything: an Uphold form on a bot-account row is a live control whose only
|
||||
// possible outcome is a 400. Flagging triggers stays available, because it writes nothing to the
|
||||
// account and is not a verdict.
|
||||
const unwiredReason = $derived(unwiredRulingReason(restriction.type));
|
||||
|
||||
let banning = $state(false);
|
||||
|
||||
// `onSubmit` picks the successor row while the current list still holds it — the reload that follows
|
||||
@@ -82,21 +90,28 @@
|
||||
{/if}
|
||||
|
||||
{#if restriction.status === 'Pending'}
|
||||
{#if unwiredReason}
|
||||
<p class="mb-3 rounded-md border border-amber-500/40 bg-amber-500/10 p-3 text-sm text-amber-200">
|
||||
{unwiredReason}
|
||||
</p>
|
||||
{/if}
|
||||
<div class="mb-3 flex flex-wrap items-center gap-2">
|
||||
<form method="POST" action="?/resolve" use:enhance={rule.enhance}>
|
||||
<input type="hidden" name="userRestrictionId" value={restriction.id} />
|
||||
<input type="hidden" name="userId" value={restriction.userId} />
|
||||
<input type="hidden" name="status" value="Upheld" />
|
||||
<Button type="submit" size="sm" variant="destructive" disabled={rule.submitting}>Uphold mute</Button>
|
||||
<Button type="submit" size="sm" variant="destructive" disabled={rule.submitting || !!unwiredReason}>Uphold mute</Button>
|
||||
</form>
|
||||
<form method="POST" action="?/resolve" use:enhance={rule.enhance}>
|
||||
<input type="hidden" name="userRestrictionId" value={restriction.id} />
|
||||
<input type="hidden" name="userId" value={restriction.userId} />
|
||||
<input type="hidden" name="status" value="Overturned" />
|
||||
<Button type="submit" size="sm" disabled={rule.submitting}>Remove mute</Button>
|
||||
<Button type="submit" size="sm" disabled={rule.submitting || !!unwiredReason}>Remove mute</Button>
|
||||
</form>
|
||||
{#if canBan}
|
||||
<Button size="sm" variant="outline" onclick={() => (banning = !banning)}>Ban user</Button>
|
||||
<!-- Disabled for the same reason, and it is the sharper case: this action bans and THEN rules,
|
||||
so on a row that cannot be ruled on it would ban the account and strand the Pending row. -->
|
||||
<Button size="sm" variant="outline" disabled={!!unwiredReason} onclick={() => (banning = !banning)}>Ban user</Button>
|
||||
{/if}
|
||||
|
||||
{#if selected.size > 0}
|
||||
|
||||
@@ -7,7 +7,14 @@
|
||||
import * as Select from '@civitai/ui/components/ui/select/index.js';
|
||||
import { urlWith } from '$lib/url';
|
||||
|
||||
let { q, status }: { q: string; status: string } = $props();
|
||||
import {
|
||||
RESTRICTION_TYPES,
|
||||
RESTRICTION_TYPE_LABELS,
|
||||
RESTRICTION_TYPE as RESTRICTION_TYPE_DEFAULT,
|
||||
type RestrictionType,
|
||||
} from '$lib/restriction-types';
|
||||
|
||||
let { q, status, type }: { q: string; status: string; type: RestrictionType } = $props();
|
||||
|
||||
// Same staging as the other filter bars: rulings reload the page, and a mirrored prop would clear a
|
||||
// search the moderator had typed but not yet submitted.
|
||||
@@ -51,6 +58,24 @@
|
||||
<Label for="restriction-q" class="text-xs text-dark-2">Username or user ID</Label>
|
||||
<Input id="restriction-q" bind:value={() => term, typed} class="mt-1 w-64" placeholder="Search…" />
|
||||
</div>
|
||||
<div>
|
||||
<Label for="restriction-type" class="text-xs text-dark-2">Type</Label>
|
||||
<!-- No "any" option: the two queues are reviewed under different assumptions, and `navigate` drops
|
||||
`page`/`selected` because both name a row in the set being replaced. -->
|
||||
<Select.Root
|
||||
type="single"
|
||||
bind:value={() => type, (v) => navigate({ type: v ?? RESTRICTION_TYPE_DEFAULT })}
|
||||
>
|
||||
<Select.Trigger id="restriction-type" class="mt-1 w-40">
|
||||
{RESTRICTION_TYPE_LABELS[type]}
|
||||
</Select.Trigger>
|
||||
<Select.Content>
|
||||
{#each RESTRICTION_TYPES as t (t)}
|
||||
<Select.Item value={t}>{RESTRICTION_TYPE_LABELS[t]}</Select.Item>
|
||||
{/each}
|
||||
</Select.Content>
|
||||
</Select.Root>
|
||||
</div>
|
||||
<div>
|
||||
<Label for="restriction-status" class="text-xs text-dark-2">Status</Label>
|
||||
<Select.Root
|
||||
|
||||
@@ -0,0 +1,244 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/**
|
||||
* The queue page's half of the restriction-type seam: which type the URL selects, and what the ruling
|
||||
* actions will accept.
|
||||
*
|
||||
* 🔴 Required, and the failure it prevents is a COLLECTION error rather than a red assertion:
|
||||
* `+page.server` → `$lib/server/query` → `users.service` → `$lib/server/db`, which demands
|
||||
* `DATABASE_REPLICA_URL` at module scope. `vitest.config.ts` withholds that variable ON PURPOSE so a
|
||||
* suite that forgets this mock throws on import rather than connecting to whatever it points at — do
|
||||
* not "fix" it by adding the variable. Unmocked, this file reports `Tests no tests` while
|
||||
* `Test Files 1 failed` carries the truth.
|
||||
*/
|
||||
vi.mock('$lib/server/db', () => ({ dbRead: {}, dbWrite: {} }));
|
||||
|
||||
const { getGenerationRestrictions, saveSuspiciousMatches } = vi.hoisted(() => ({
|
||||
getGenerationRestrictions: vi.fn(),
|
||||
saveSuspiciousMatches: vi.fn(),
|
||||
}));
|
||||
const { resolveRestriction, setBanned, banConfirmed } = vi.hoisted(() => ({
|
||||
resolveRestriction: vi.fn(),
|
||||
setBanned: vi.fn(),
|
||||
banConfirmed: vi.fn(),
|
||||
}));
|
||||
|
||||
// Partial: the real module owns the type vocabulary this file is about, and re-declaring it here would
|
||||
// let the constants drift from the page under test while every assertion still passed.
|
||||
vi.mock('$lib/server/user-restriction.service', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('$lib/server/user-restriction.service')>()),
|
||||
getGenerationRestrictions,
|
||||
saveSuspiciousMatches,
|
||||
}));
|
||||
vi.mock('$lib/server/user-actions.service', () => ({
|
||||
resolveRestriction,
|
||||
setBanned,
|
||||
banConfirmed,
|
||||
}));
|
||||
vi.mock('$lib/server/access', () => ({
|
||||
requiresGrant:
|
||||
(_grant: string, fn: unknown) =>
|
||||
(...args: unknown[]) =>
|
||||
(fn as (...a: unknown[]) => unknown)(...args),
|
||||
}));
|
||||
|
||||
const { load, actions } = await import('../+page.server');
|
||||
const { RESTRICTION_TYPES, RULINGS_WIRED_FOR, unwiredRulingReason } = await import(
|
||||
'$lib/restriction-types'
|
||||
);
|
||||
|
||||
type LoadResult = { type: string; items: unknown[] };
|
||||
const runLoad = (search = '') =>
|
||||
(load as unknown as (e: { url: URL }) => Promise<LoadResult>)({
|
||||
url: new URL(`https://moderator.example/audit/generator-restrictions${search}`),
|
||||
});
|
||||
|
||||
const formEvent = (fields: Record<string, string>) => {
|
||||
const data = new FormData();
|
||||
for (const [k, v] of Object.entries(fields)) data.append(k, v);
|
||||
return {
|
||||
request: { formData: async () => data },
|
||||
locals: { user: { id: 7 } },
|
||||
} as unknown as Parameters<(typeof actions)['resolve']>[0];
|
||||
};
|
||||
|
||||
const row = (over: Record<string, unknown> = {}) => ({
|
||||
id: 5,
|
||||
userId: 42,
|
||||
username: 'someone',
|
||||
type: 'generation',
|
||||
status: 'Pending',
|
||||
createdAt: new Date(),
|
||||
resolvedAt: null,
|
||||
resolvedMessage: null,
|
||||
userMessage: null,
|
||||
userMessageAt: null,
|
||||
triggers: [],
|
||||
...over,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
getGenerationRestrictions.mockResolvedValue({ items: [], totalCount: 0 });
|
||||
resolveRestriction.mockResolvedValue({ ok: true });
|
||||
setBanned.mockResolvedValue({ ok: true });
|
||||
banConfirmed.mockResolvedValue(true);
|
||||
});
|
||||
|
||||
/** The `type` the page asked the query layer for, on its list read. */
|
||||
const requestedType = () => getGenerationRestrictions.mock.calls[0][0].type;
|
||||
|
||||
describe('generator-restrictions load — type', () => {
|
||||
it('opens on generation when the URL names no type', async () => {
|
||||
const data = await runLoad();
|
||||
|
||||
expect(requestedType()).toBe('generation');
|
||||
expect(data.type).toBe('generation');
|
||||
});
|
||||
|
||||
it('opens on the type the URL names', async () => {
|
||||
const data = await runLoad('?type=bot-account');
|
||||
|
||||
expect(requestedType()).toBe('bot-account');
|
||||
expect(data.type).toBe('bot-account');
|
||||
});
|
||||
|
||||
// 🔴 `.catch()` rather than a rejection, matching the other filters on this page: a stale bookmark
|
||||
// should open the default queue, not a 500. The hazard it removes is the URL reaching the query layer
|
||||
// as an arbitrary string — including `any`, which the service treats as "drop the filter" and would
|
||||
// render every type's rows in one list.
|
||||
it.each(['?type=any', '?type=nonsense', '?type=', '?type=GENERATION'])(
|
||||
'falls back to generation for %s',
|
||||
async (search) => {
|
||||
const data = await runLoad(search);
|
||||
|
||||
expect(requestedType()).toBe('generation');
|
||||
expect(data.type).toBe('generation');
|
||||
}
|
||||
);
|
||||
|
||||
it('still passes the other filters through untouched', async () => {
|
||||
await runLoad('?type=bot-account&status=Upheld&q=someone&page=3');
|
||||
|
||||
expect(getGenerationRestrictions).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: 'bot-account',
|
||||
status: 'Upheld',
|
||||
username: 'someone',
|
||||
page: 3,
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('generator-restrictions actions — ruling scope', () => {
|
||||
/**
|
||||
* 🔴 A verdict is still generation-shaped. `resolveUserRestriction` in the main app hardcodes the
|
||||
* `generation-restriction-upheld` / `-overturned` notification types, a `moderator:generationRestriction*`
|
||||
* update source and a generation-worded email, and on an overturn it resets the PROMPT violation
|
||||
* counter. Ruling on a bot-account row through it would tell the user their generation access was
|
||||
* restored over something that has nothing to do with generation.
|
||||
*
|
||||
* Enforced server-side rather than by hiding a button, because the check has to hold against a posted
|
||||
* id and not merely against what the page chose to render.
|
||||
*/
|
||||
it('refuses to resolve a restriction whose type has no verdict path', async () => {
|
||||
getGenerationRestrictions.mockResolvedValue({
|
||||
items: [row({ type: 'bot-account' })],
|
||||
totalCount: 1,
|
||||
});
|
||||
|
||||
const result = (await actions.resolve(
|
||||
formEvent({ userRestrictionId: '5', status: 'Upheld' })
|
||||
)) as { status: number; data: { error: string } };
|
||||
|
||||
expect(result.status).toBe(400);
|
||||
expect(result.data.error).toMatch(/not yet available for "bot-account"/);
|
||||
expect(resolveRestriction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still resolves a generation restriction', async () => {
|
||||
getGenerationRestrictions.mockResolvedValue({ items: [row()], totalCount: 1 });
|
||||
|
||||
const result = await actions.resolve(formEvent({ userRestrictionId: '5', status: 'Upheld' }));
|
||||
|
||||
expect(result).toEqual({ success: true });
|
||||
expect(resolveRestriction).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ userRestrictionId: 5, status: 'Upheld', userId: 42 })
|
||||
);
|
||||
});
|
||||
|
||||
// The ban action bans and THEN rules. Refusing only at the ruling would leave the account banned
|
||||
// against a restriction that cannot be resolved — the stranded Pending row that handler exists to
|
||||
// avoid — so the check has to come first.
|
||||
it('refuses to ban off a restriction whose type has no verdict path, without banning', async () => {
|
||||
getGenerationRestrictions.mockResolvedValue({
|
||||
items: [row({ type: 'bot-account' })],
|
||||
totalCount: 1,
|
||||
});
|
||||
|
||||
// `reasonCode` is omitted deliberately — it is optional, so this payload is VALID. An invalid one
|
||||
// is also answered `400` with `setBanned` untouched, which made an earlier version of this test
|
||||
// pass against pre-change code for a reason that had nothing to do with the type. The message is
|
||||
// asserted for the same reason: `400` alone cannot tell the two refusals apart.
|
||||
const result = (await actions.ban(formEvent({ userRestrictionId: '5' }))) as {
|
||||
status: number;
|
||||
data: { error: string };
|
||||
};
|
||||
|
||||
expect(result.status).toBe(400);
|
||||
expect(result.data.error).toMatch(/not yet available for "bot-account"/);
|
||||
expect(setBanned).not.toHaveBeenCalled();
|
||||
expect(resolveRestriction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still bans off a generation restriction', async () => {
|
||||
// The positive control for the refusal above: the same payload, differing only in the row's type,
|
||||
// must go all the way through. Without it the refusal could be rejecting every ban.
|
||||
getGenerationRestrictions.mockResolvedValue({ items: [row()], totalCount: 1 });
|
||||
|
||||
const result = await actions.ban(formEvent({ userRestrictionId: '5' }));
|
||||
|
||||
expect(result).toEqual({ success: true });
|
||||
expect(setBanned).toHaveBeenCalledWith(expect.objectContaining({ userId: 42, ban: true }));
|
||||
expect(resolveRestriction).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ userRestrictionId: 5, status: 'Upheld' })
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* An INVARIANT GUARD — this passed before the route stopped spelling its own message, and is
|
||||
* recorded as such rather than counted as regression coverage.
|
||||
*
|
||||
* What it pins is that the route READS the shared predicate rather than carrying a second copy. The
|
||||
* refusal that actually protects the account now lives in the main app's `resolveUserRestriction`
|
||||
* (this route posts through it), and the two are pinned to each other by the seam test — but only if
|
||||
* this end of the chain is the shared list and not a private one that happens to agree today.
|
||||
*/
|
||||
it.each(RESTRICTION_TYPES.filter((t) => !RULINGS_WIRED_FOR.includes(t)))(
|
||||
'refuses %s with the shared predicate’s own words, not a second copy of them',
|
||||
async (type) => {
|
||||
getGenerationRestrictions.mockResolvedValue({ items: [row({ type })], totalCount: 1 });
|
||||
|
||||
const result = (await actions.resolve(
|
||||
formEvent({ userRestrictionId: '5', status: 'Upheld' })
|
||||
)) as { status: number; data: { error: string } };
|
||||
|
||||
expect(result.data.error).toBe(unwiredRulingReason(type));
|
||||
// Non-vacuous: `toBe(null)` would also pass if the action had returned success.
|
||||
expect(unwiredRulingReason(type)).not.toBeNull();
|
||||
}
|
||||
);
|
||||
|
||||
// A by-id lookup must not be filtered by the default type: a form posts to `?/resolve`, which
|
||||
// replaces the query string, so the action cannot know which queue the row came from.
|
||||
it('looks a restriction up across every type', async () => {
|
||||
getGenerationRestrictions.mockResolvedValue({ items: [row()], totalCount: 1 });
|
||||
|
||||
await actions.resolve(formEvent({ userRestrictionId: '5', status: 'Upheld' }));
|
||||
|
||||
expect(getGenerationRestrictions).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ restrictionId: 5, type: 'any' })
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -10,6 +10,7 @@
|
||||
import { fetchSupport } from './user-support';
|
||||
import { REWARDS_ELIGIBILITY } from './enforcement-options';
|
||||
import { FormState } from '$lib/form-state.svelte';
|
||||
import { unwiredRulingReason } from '$lib/restriction-types';
|
||||
import ErrorAlert from '$lib/components/ErrorAlert.svelte';
|
||||
|
||||
type Identity = NonNullable<LayoutData['result']>['identity'];
|
||||
@@ -42,6 +43,13 @@
|
||||
const support = $derived(browser ? fetchSupport(identity.id, version) : null);
|
||||
const mutesUrl = $derived(userLookupUrl(identity.id, 'mutes'));
|
||||
|
||||
// A restriction of a type with no verdict path cannot be ruled on ANYWHERE — `resolveUserRestriction`
|
||||
// in the main app refuses it, which is what this panel posts through. Without this the panel offered
|
||||
// Overturn/Uphold on such a row, and clicking Overturn would have sent a "your generation access has
|
||||
// been restored" notice and reset the account's prompt-violation counter over an unrelated case.
|
||||
// Falls back to `generation` for a null type, matching the label below.
|
||||
const unwiredReason = $derived(unwiredRulingReason(identity.restrictionType ?? 'generation'));
|
||||
|
||||
// `reload: true` because these writes change `identity`, which DOES come from `load` — unlike the
|
||||
// panels fed by `/api/*`, where reloading re-runs the reaction scan for nothing.
|
||||
//
|
||||
@@ -87,6 +95,14 @@
|
||||
A <strong>{identity.restrictionType ?? 'generation'}</strong> restriction on this account is
|
||||
awaiting a ruling. Unmuting alone leaves it Pending — rule on it here instead.
|
||||
</p>
|
||||
{#if unwiredReason}
|
||||
<p class="mb-2 text-sm text-amber-200">
|
||||
{unwiredReason} Review it in the
|
||||
<a href="/audit/generator-restrictions?type={identity.restrictionType}" class={LINK_CLASS}>
|
||||
restriction queue
|
||||
</a>.
|
||||
</p>
|
||||
{/if}
|
||||
<form method="POST" action="?/resolveRestriction" use:enhance={form.enhance} class="grid gap-2">
|
||||
<input type="hidden" name="userRestrictionId" value={identity.restrictionId} />
|
||||
<input type="hidden" name="userId" value={identity.id} />
|
||||
@@ -97,7 +113,13 @@
|
||||
/>
|
||||
<div class="flex flex-wrap gap-2">
|
||||
<!-- One field, two submits: a submit button contributes a single name/value pair. -->
|
||||
<Button type="submit" name="status" value="Overturned" size="sm" disabled={form.submitting}>
|
||||
<Button
|
||||
type="submit"
|
||||
name="status"
|
||||
value="Overturned"
|
||||
size="sm"
|
||||
disabled={form.submitting || !!unwiredReason}
|
||||
>
|
||||
Overturn — lift it
|
||||
</Button>
|
||||
<Button
|
||||
@@ -106,7 +128,7 @@
|
||||
value="Upheld"
|
||||
size="sm"
|
||||
variant="destructive"
|
||||
disabled={form.submitting}
|
||||
disabled={form.submitting || !!unwiredReason}
|
||||
>
|
||||
Uphold — keep them muted
|
||||
</Button>
|
||||
|
||||
@@ -27,8 +27,21 @@ import {
|
||||
*
|
||||
* The driver ignores the SQL and answers every query with the same `rows`, so the count a chain sees is
|
||||
* fixed here and never by the query's own LIMIT.
|
||||
*
|
||||
* 🔴 **Pass `params` when the VALUE bound to a predicate is the thing under test.** The SQL text alone
|
||||
* cannot see it: `where('ur.type','=',x)` compiles to `"ur"."type" = $1` for every `x`, so a mutant that
|
||||
* ignores its argument and hardcodes a literal emits byte-identical SQL and survives an assertion over
|
||||
* `sql`. `params[i]` holds the parameters of `sql[i]`.
|
||||
*
|
||||
* `params` is a MUTABLE `unknown[][]` and says so. Being written is the parameter's whole purpose, and
|
||||
* a `readonly` annotation that has to be cast away at the one place it is used is a contract the code
|
||||
* contradicts — it reads as a promise to callers that this function does not touch their array.
|
||||
*/
|
||||
export function capturingDb(sql: string[], rows: unknown[] = []): Kysely<never> {
|
||||
export function capturingDb(
|
||||
sql: string[],
|
||||
rows: unknown[] = [],
|
||||
params?: unknown[][]
|
||||
): Kysely<never> {
|
||||
class CannedRowDriver extends DummyDriver {
|
||||
async acquireConnection(): Promise<DatabaseConnection> {
|
||||
return {
|
||||
@@ -50,7 +63,10 @@ export function capturingDb(sql: string[], rows: unknown[] = []): Kysely<never>
|
||||
createQueryCompiler: () => new PostgresQueryCompiler(),
|
||||
},
|
||||
log: (e) => {
|
||||
if (e.level === 'query') sql.push(e.query.sql);
|
||||
if (e.level !== 'query') return;
|
||||
sql.push(e.query.sql);
|
||||
// Pushed in the same branch so the two arrays stay index-aligned by construction.
|
||||
params?.push([...e.query.parameters]);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import path from 'node:path';
|
||||
import type { NextApiResponse } from 'next';
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type * as NotificationService from '~/server/services/notification.service';
|
||||
import type * as SessionInvalidation from '~/server/auth/session-invalidation';
|
||||
import type * as ModeratorService from '~/server/services/moderator.service';
|
||||
@@ -89,17 +91,25 @@ const {
|
||||
return row ? { id: row.id } : null;
|
||||
}
|
||||
),
|
||||
findUnique: vi.fn(async ({ where }: { where: { id: number } }) => {
|
||||
const row = store.restrictions.find((r) => r.id === where.id);
|
||||
if (!row) return null;
|
||||
const user = store.users.get(row.userId);
|
||||
return {
|
||||
id: row.id,
|
||||
userId: row.userId,
|
||||
status: row.status,
|
||||
user: user ? { email: user.email, username: user.username } : null,
|
||||
};
|
||||
}),
|
||||
// 🔴 Honours `select` for `type` alone, unlike every other read in this fake. The ruling-scope
|
||||
// suite turns on the service having asked for it: a version that refuses non-generation rows but
|
||||
// forgets `type: true` in its `select` reads `undefined` from the real Prisma client and either
|
||||
// refuses everything or nothing. A fake that answered with the column regardless would hide that
|
||||
// entirely — the test would pass against code that cannot work.
|
||||
findUnique: vi.fn(
|
||||
async ({ where, select }: { where: { id: number }; select?: Record<string, unknown> }) => {
|
||||
const row = store.restrictions.find((r) => r.id === where.id);
|
||||
if (!row) return null;
|
||||
const user = store.users.get(row.userId);
|
||||
return {
|
||||
id: row.id,
|
||||
userId: row.userId,
|
||||
status: row.status,
|
||||
...(select?.type ? { type: row.type } : {}),
|
||||
user: user ? { email: user.email, username: user.username } : null,
|
||||
};
|
||||
}
|
||||
),
|
||||
update: vi.fn(
|
||||
async ({ where, data }: { where: { id: number }; data: Record<string, unknown> }) => {
|
||||
const row = store.restrictions.find((r) => r.id === where.id);
|
||||
@@ -183,8 +193,18 @@ import { setUserMuted } from '~/server/services/user.service';
|
||||
import {
|
||||
applyPendingReviewMute,
|
||||
buildManualMuteTriggers,
|
||||
PENDING_REVIEW_MUTE_NOTIFICATION,
|
||||
RULINGS_WIRED_FOR,
|
||||
USER_RESTRICTION_TYPES,
|
||||
unwiredRulingReason,
|
||||
type UserRestrictionType,
|
||||
} from '~/server/services/user-restriction.service';
|
||||
import { overturnPendingReviewMute } from '~/server/services/user-restriction-resolve.service';
|
||||
import {
|
||||
overturnPendingReviewMute,
|
||||
resolveUserRestriction,
|
||||
} from '~/server/services/user-restriction-resolve.service';
|
||||
import { handleEndpointError } from '~/server/utils/endpoint-helpers';
|
||||
import { UserRestrictionStatus } from '~/shared/utils/prisma/enums';
|
||||
|
||||
const USER_ID = 101;
|
||||
const MOD_ID = 102;
|
||||
@@ -365,6 +385,572 @@ describe('pending-review mute', () => {
|
||||
expect(result).toMatchObject({ muted: true });
|
||||
expect(store.users.get(USER_ID)).toMatchObject({ muted: true });
|
||||
});
|
||||
|
||||
it('notifies the user that generation access is restricted', async () => {
|
||||
const { userRestrictionId } = (await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
})) as { userRestrictionId: number };
|
||||
|
||||
// Pinned whole rather than by `objectContaining`: the key is the notification service's dedupe
|
||||
// handle, so a change to its shape re-notifies every already-notified user.
|
||||
expect(createNotification).toHaveBeenCalledExactlyOnceWith({
|
||||
type: 'generation-muted',
|
||||
key: `generation-muted:${USER_ID}:${userRestrictionId}`,
|
||||
category: 'System',
|
||||
userId: USER_ID,
|
||||
details: {},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The seam a bot-account detector files through. Nothing raises a non-generation restriction yet — this
|
||||
* is the parameter that lets one, and the properties below are what keep it from cannibalising the
|
||||
* queue that already exists.
|
||||
*/
|
||||
describe('pending-review mute — restriction type', () => {
|
||||
beforeEach(seed);
|
||||
|
||||
it('files a generation restriction when no type is given', async () => {
|
||||
await applyPendingReviewMute({ userId: USER_ID, triggers, updateSource: 'test' });
|
||||
|
||||
expect(store.restrictions).toHaveLength(1);
|
||||
expect(store.restrictions[0]).toMatchObject({ type: 'generation', status: 'Pending' });
|
||||
});
|
||||
|
||||
it('files a restriction of the type it was given', async () => {
|
||||
await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(store.restrictions).toHaveLength(1);
|
||||
expect(store.restrictions[0]).toMatchObject({
|
||||
userId: USER_ID,
|
||||
type: 'bot-account',
|
||||
status: 'Pending',
|
||||
});
|
||||
expect(store.restrictions[0].triggers).toEqual(triggers);
|
||||
});
|
||||
|
||||
// 🔴 The pair below is the point of the whole change. Dedupe reads "this user already has an open
|
||||
// case", and scoped to the user alone it means the FIRST queue to mute someone permanently silences
|
||||
// every other queue for that account — a detector's findings would return `deduped: true` against a
|
||||
// row about something else entirely, and file nothing a moderator could ever see.
|
||||
it('does not let an open generation case swallow a bot-account mute', async () => {
|
||||
const first = await applyPendingReviewMute({ userId: USER_ID, triggers, updateSource: 'test' });
|
||||
const second = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(second).toMatchObject({ muted: true, deduped: false });
|
||||
expect((second as { userRestrictionId: number }).userRestrictionId).not.toBe(
|
||||
(first as { userRestrictionId: number }).userRestrictionId
|
||||
);
|
||||
expect(store.restrictions.map((r) => r.type)).toEqual(['generation', 'bot-account']);
|
||||
});
|
||||
|
||||
it('does not let an open bot-account case swallow a generation mute', async () => {
|
||||
const first = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
const second = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
});
|
||||
|
||||
expect(second).toMatchObject({ muted: true, deduped: false });
|
||||
expect((second as { userRestrictionId: number }).userRestrictionId).not.toBe(
|
||||
(first as { userRestrictionId: number }).userRestrictionId
|
||||
);
|
||||
expect(store.restrictions.map((r) => r.type)).toEqual(['bot-account', 'generation']);
|
||||
});
|
||||
|
||||
it('still dedupes within a type, so a retry files nothing new', async () => {
|
||||
const first = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
const second = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(store.restrictions).toHaveLength(1);
|
||||
expect(second).toEqual({
|
||||
muted: true,
|
||||
userRestrictionId: (first as { userRestrictionId: number }).userRestrictionId,
|
||||
deduped: true,
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 `createNotification` validates `type` against NOTHING — `z.string()` at the schema, `text` at
|
||||
* both tables, and the fan-out worker inserts it verbatim. An unregistered type is persisted and
|
||||
* increments the user's unread badge, while the bell dropdown drops it at render, leaving a phantom
|
||||
* count with no click target. And `generation-muted` reads "your generation access has been
|
||||
* restricted", which is a lie about a bot-account mute. So a type with no notification of its own
|
||||
* sends none until someone registers one.
|
||||
*/
|
||||
it('sends no notification for a type that has none mapped', async () => {
|
||||
await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(PENDING_REVIEW_MUTE_NOTIFICATION['bot-account']).toBeNull();
|
||||
expect(createNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('mutes the account and refreshes the session for a non-generation type all the same', async () => {
|
||||
const result = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ muted: true });
|
||||
expect(store.users.get(USER_ID)).toMatchObject({ muted: true });
|
||||
expect(refreshSession).toHaveBeenCalledWith(USER_ID, { caller: 'moderation' });
|
||||
});
|
||||
|
||||
it('writes the mute and a typed restriction in one transaction', async () => {
|
||||
await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(dbWrite.$transaction).toHaveBeenCalledOnce();
|
||||
expect(dbWrite.$transaction.mock.calls[0][0]).toHaveLength(2);
|
||||
});
|
||||
|
||||
it.each(['generation', 'bot-account'] as const)(
|
||||
'never writes mutedAt for a %s restriction',
|
||||
async (type) => {
|
||||
await applyPendingReviewMute({ userId: USER_ID, triggers, updateSource: 'test', type });
|
||||
|
||||
const dataArgs = dbWrite.user.update.mock.calls.map(([arg]) => arg.data);
|
||||
expect(dataArgs).toEqual([{ muted: true }]);
|
||||
expect(store.users.get(USER_ID)).toMatchObject({ muted: true, mutedAt: null });
|
||||
}
|
||||
);
|
||||
|
||||
it.each([
|
||||
['moderator', MOD_ID, 'moderator'],
|
||||
['banned user', BANNED_ID, 'banned'],
|
||||
['deleted user', DELETED_ID, 'deleted'],
|
||||
['the official brand account', constants.system.officialUserId, 'protected'],
|
||||
['the system actor', constants.system.user.id, 'protected'],
|
||||
])('refuses to file a bot-account restriction against a %s', async (_label, userId, skipped) => {
|
||||
const result = await applyPendingReviewMute({
|
||||
userId,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(result).toEqual({ muted: false, skipped });
|
||||
expect(store.restrictions).toHaveLength(0);
|
||||
expect(store.users.get(userId)?.muted ?? false).toBe(false);
|
||||
expect(createNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('repairs an unmuted user holding an open case of the SAME type only', async () => {
|
||||
store.restrictions.push({
|
||||
id: 99,
|
||||
userId: USER_ID,
|
||||
type: 'bot-account',
|
||||
status: 'Pending',
|
||||
triggers: [],
|
||||
createdAt: new Date(),
|
||||
});
|
||||
|
||||
const result = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: 'bot-account',
|
||||
});
|
||||
|
||||
expect(result).toEqual({ muted: true, userRestrictionId: 99, deduped: true });
|
||||
expect(store.users.get(USER_ID)).toMatchObject({ muted: true, mutedAt: null });
|
||||
expect(store.restrictions).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The runtime guard, and what it is actually for. No HTTP boundary supplies this parameter today:
|
||||
* neither production caller passes a `type`, and `mute-user-pending-review.ts`'s zod schema has no
|
||||
* `type` key, so no request body can reach it. The guard is there for the shape of the NEXT caller —
|
||||
* this seam exists so a detector can file into the queue, and the obvious wiring is a route
|
||||
* forwarding a JSON field — and for the callers TypeScript already cannot vouch for: an `as` cast, a
|
||||
* value read back off the free-text `UserRestriction.type` column, a JS caller.
|
||||
*/
|
||||
describe('pending-review mute — type is validated at runtime', () => {
|
||||
beforeEach(seed);
|
||||
|
||||
it.each([
|
||||
['a near miss', 'bot-acount'],
|
||||
['a plausible-looking new kind', 'spam-account'],
|
||||
['an empty string', ''],
|
||||
])('refuses %s and mutes nobody', async (_label, type) => {
|
||||
await expect(
|
||||
applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type: type as UserRestrictionType,
|
||||
})
|
||||
).rejects.toThrow(`Unknown user restriction type "${type}"`);
|
||||
|
||||
// The harm the throw prevents, spelled out: an out-of-vocabulary value used to MUTE the account,
|
||||
// file a row the queue's `z.enum(...).catch(...)` can never select, and — the notification map
|
||||
// returning `undefined` for it — tell the user nothing. A silently muted user, no reviewable case.
|
||||
expect(store.users.get(USER_ID)).toMatchObject({ muted: false, mutedAt: null });
|
||||
expect(store.restrictions).toHaveLength(0);
|
||||
expect(dbWrite.$transaction).not.toHaveBeenCalled();
|
||||
expect(createNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The positive control. Without it the guard above could be rejecting every type, and the suite
|
||||
// would still be green — `it.each` over the real vocabulary is what makes the refusal specific.
|
||||
it.each(USER_RESTRICTION_TYPES)('accepts %s', async (type) => {
|
||||
const result = await applyPendingReviewMute({
|
||||
userId: USER_ID,
|
||||
triggers,
|
||||
updateSource: 'test',
|
||||
type,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ muted: true });
|
||||
expect(store.restrictions).toHaveLength(1);
|
||||
expect(store.restrictions[0].type).toBe(type);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 Finding 1 of the adversarial audit on #4609, closed one level BELOW the routes.
|
||||
*
|
||||
* `resolveUserRestriction` is the single write path for a verdict, and everything it does is
|
||||
* generation-shaped: the `generation-restriction-upheld` / `-overturned` notification types, a
|
||||
* `moderator:generationRestriction*` update source, a generation-worded email, and — on an overturn —
|
||||
* `resetProhibitedRequestCount`, which wipes the account's real PROMPT-violation counter.
|
||||
*
|
||||
* Five callers reach it: the tRPC router, `/api/mod/restriction/resolve` (which is what BOTH moderator
|
||||
* ruling surfaces post through — the audit queue AND the retool User Lookup panel), and
|
||||
* `overturnPendingReviewMute`. Only the audit queue checked the type, so three of those five would have
|
||||
* run the whole generation-shaped sequence against a bot-account row. The check lives here now, which
|
||||
* is why these tests address the SERVICE rather than any one route.
|
||||
*/
|
||||
describe('resolveUserRestriction — ruling scope', () => {
|
||||
beforeEach(seed);
|
||||
|
||||
const fileRestriction = (type: string, status = 'Pending') => {
|
||||
store.restrictions.push({
|
||||
id: 1,
|
||||
userId: USER_ID,
|
||||
type,
|
||||
status,
|
||||
triggers: [],
|
||||
createdAt: new Date(),
|
||||
});
|
||||
return 1;
|
||||
};
|
||||
|
||||
it.each([UserRestrictionStatus.Overturned, UserRestrictionStatus.Upheld] as const)(
|
||||
'refuses to %s a restriction whose type has no verdict path',
|
||||
async (status) => {
|
||||
const id = fileRestriction('bot-account');
|
||||
|
||||
await expect(
|
||||
resolveUserRestriction({ userRestrictionId: id, status, moderatorId: MOD_ID })
|
||||
).rejects.toThrow('Rulings are not yet available for "bot-account" restrictions');
|
||||
|
||||
// Nothing at all happened — checked rather than assumed, because the refusal is only worth
|
||||
// anything if it lands BEFORE the first write.
|
||||
expect(dbWrite.userRestriction.update).not.toHaveBeenCalled();
|
||||
expect(store.restrictions[0].status).toBe('Pending');
|
||||
expect(dbWrite.user.update).not.toHaveBeenCalled();
|
||||
expect(createNotification).not.toHaveBeenCalled();
|
||||
expect(cancelSubscription).not.toHaveBeenCalled();
|
||||
expect(reinstateSubscription).not.toHaveBeenCalled();
|
||||
// The one with a lasting cost: this counter is the account's real prompt-violation history, and
|
||||
// an overturn on an unrelated case used to reset it to zero.
|
||||
expect(resetProhibitedRequestCount).not.toHaveBeenCalled();
|
||||
}
|
||||
);
|
||||
|
||||
/**
|
||||
* The positive control for the pair above, and it is doing more work than it looks: the fixture rows
|
||||
* differ ONLY in `type`. Without it the refusal could be rejecting every ruling — which is exactly
|
||||
* what happens if the service stops selecting `type` and reads `undefined`.
|
||||
*/
|
||||
it('still overturns a generation restriction, with every side effect intact', async () => {
|
||||
const id = fileRestriction('generation');
|
||||
|
||||
const result = await resolveUserRestriction({
|
||||
userRestrictionId: id,
|
||||
status: UserRestrictionStatus.Overturned,
|
||||
moderatorId: MOD_ID,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ userId: USER_ID });
|
||||
expect(store.restrictions[0]).toMatchObject({ status: 'Overturned', resolvedBy: MOD_ID });
|
||||
expect(reinstateSubscription).toHaveBeenCalledWith({ userId: USER_ID });
|
||||
expect(resetProhibitedRequestCount).toHaveBeenCalledWith(USER_ID);
|
||||
expect(createNotification).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ type: 'generation-restriction-overturned' })
|
||||
);
|
||||
});
|
||||
|
||||
it('still upholds a generation restriction, with every side effect intact', async () => {
|
||||
const id = fileRestriction('generation');
|
||||
|
||||
await resolveUserRestriction({
|
||||
userRestrictionId: id,
|
||||
status: UserRestrictionStatus.Upheld,
|
||||
moderatorId: MOD_ID,
|
||||
});
|
||||
|
||||
expect(store.restrictions[0]).toMatchObject({ status: 'Upheld' });
|
||||
expect(store.users.get(USER_ID)?.mutedAt).toBeInstanceOf(Date);
|
||||
expect(cancelSubscription).toHaveBeenCalledWith({ userId: USER_ID, atPeriodEnd: true });
|
||||
expect(createNotification).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ type: 'generation-restriction-upheld' })
|
||||
);
|
||||
});
|
||||
|
||||
// The refusal precedes the already-resolved check, so a row this path cannot rule on reports the
|
||||
// reason it cannot rather than an argument about its status.
|
||||
it('refuses an unwired type before it argues about the status', async () => {
|
||||
const id = fileRestriction('bot-account', 'Upheld');
|
||||
|
||||
await expect(
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: id,
|
||||
status: UserRestrictionStatus.Overturned,
|
||||
moderatorId: MOD_ID,
|
||||
})
|
||||
).rejects.toThrow('Rulings are not yet available for "bot-account" restrictions');
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The SEAM between the refusal and what a moderator actually reads.
|
||||
*
|
||||
* Both ruling surfaces post through `/api/mod/restriction/resolve`, whose `defineModeratorEndpoint`
|
||||
* wrapper hands a throw to `handleEndpointError`. A plain `Error` falls to that helper's catch-all
|
||||
* branch and reaches the wire as **500 "An unexpected error occurred"** — the retool panel then
|
||||
* renders "Restriction ruling: An unexpected error occurred." and the reason is destroyed. So the
|
||||
* service throwing the right words is only half the behaviour; these drive the REAL helper over the
|
||||
* REAL thrown value, because a test that asserted only the message would stay green through exactly
|
||||
* that 500.
|
||||
*/
|
||||
describe('the refusal survives the REST envelope', () => {
|
||||
/**
|
||||
* The moderator app's REAL body reader, loaded across the app boundary by filesystem path. It is
|
||||
* import-free for exactly this reason — see `apps/moderator/src/lib/server/rest-error-reason.ts`
|
||||
* and the note in `moderator-restriction-vocabulary.harness.ts` about the same coupling
|
||||
* (resolving a path into `apps/moderator` needs `svelte-kit sync` to have run there).
|
||||
*/
|
||||
let restErrorReason: (body: unknown, status: number) => string | null;
|
||||
|
||||
beforeAll(async () => {
|
||||
const file = path.resolve(
|
||||
__dirname,
|
||||
'../../..',
|
||||
'apps/moderator/src/lib/server/rest-error-reason.ts'
|
||||
);
|
||||
({ restErrorReason } = await import(/* @vite-ignore */ file));
|
||||
// The import resolving is not the same as it being the thing we meant to load.
|
||||
expect(typeof restErrorReason).toBe('function');
|
||||
});
|
||||
|
||||
const throughRest = async (fn: () => Promise<unknown>) => {
|
||||
const res = createRes();
|
||||
let threw = false;
|
||||
try {
|
||||
await fn();
|
||||
} catch (e) {
|
||||
threw = true;
|
||||
handleEndpointError(res as unknown as NextApiResponse, e);
|
||||
}
|
||||
// Positive control: a call that did NOT throw would leave `state` at its zero value and every
|
||||
// assertion below would be about the fake rather than about the error.
|
||||
expect(threw).toBe(true);
|
||||
return res.state as { status: number; body: Record<string, unknown> & { message?: string } };
|
||||
};
|
||||
|
||||
it('reaches REST as a 400 naming the type, not an opaque 500', async () => {
|
||||
const id = fileRestriction('bot-account');
|
||||
|
||||
const { status, body } = await throughRest(() =>
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: id,
|
||||
status: UserRestrictionStatus.Overturned,
|
||||
moderatorId: MOD_ID,
|
||||
})
|
||||
);
|
||||
|
||||
expect(status).toBe(400);
|
||||
expect(body.message).toContain(
|
||||
'Rulings are not yet available for "bot-account" restrictions'
|
||||
);
|
||||
// The exact sentence the moderator used to get instead. Pinned by value: it is the observable
|
||||
// that says the reason was destroyed rather than merely reworded.
|
||||
expect(body.message).not.toContain('An unexpected error occurred');
|
||||
});
|
||||
|
||||
it('reports a missing row as a 404 rather than a server fault', async () => {
|
||||
const { status, body } = await throughRest(() =>
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: 4242,
|
||||
status: UserRestrictionStatus.Upheld,
|
||||
moderatorId: MOD_ID,
|
||||
})
|
||||
);
|
||||
|
||||
expect(status).toBe(404);
|
||||
expect(body.message).toBe('Restriction record not found');
|
||||
});
|
||||
|
||||
it('reports an already-ruled row as a 400 rather than a server fault', async () => {
|
||||
const id = fileRestriction('generation', 'Upheld');
|
||||
|
||||
const { status, body } = await throughRest(() =>
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: id,
|
||||
status: UserRestrictionStatus.Overturned,
|
||||
moderatorId: MOD_ID,
|
||||
})
|
||||
);
|
||||
|
||||
expect(status).toBe(400);
|
||||
expect(body.message).toBe('Restriction has already been resolved');
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The assertions above are about the WIRE. This one is about what the only in-repo consumer
|
||||
* gets out of it, and the two are NOT the same claim — which is how the gap below survived a
|
||||
* green suite for a whole round.
|
||||
*
|
||||
* `handleEndpointError`'s 4xx pass-through emits `{ message }` and no `error` key, while every
|
||||
* other refusal from `defineModeratorEndpoint` emits `{ error, message, code }`. The moderator
|
||||
* app's `readError` read `body.error` and nothing else, so all three refusals above came back
|
||||
* `null` and the operator saw `"Restriction ruling returned 400."` — the reason destroyed again,
|
||||
* one layer further out than the opaque 500 this endpoint change removed.
|
||||
*
|
||||
* So this drives the REAL emitter into the REAL reader in one process. Both halves mocked
|
||||
* separately is exactly the arrangement that cannot see a disagreement about the field name:
|
||||
* `restErrorReason` is loaded across the app boundary by filesystem path, the same mechanism (and
|
||||
* the same import-free precondition) as the vocabulary harness.
|
||||
*/
|
||||
it('hands the moderator app a reason it can read, not just a status', async () => {
|
||||
const cases: [string, () => Promise<unknown>, number, string][] = [
|
||||
[
|
||||
'an unwired type',
|
||||
() =>
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: fileRestriction('bot-account'),
|
||||
status: UserRestrictionStatus.Overturned,
|
||||
moderatorId: MOD_ID,
|
||||
}),
|
||||
400,
|
||||
'Rulings are not yet available for "bot-account" restrictions',
|
||||
],
|
||||
[
|
||||
'a missing row',
|
||||
() =>
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: 4242,
|
||||
status: UserRestrictionStatus.Upheld,
|
||||
moderatorId: MOD_ID,
|
||||
}),
|
||||
404,
|
||||
'Restriction record not found',
|
||||
],
|
||||
[
|
||||
'an already-ruled row',
|
||||
() =>
|
||||
resolveUserRestriction({
|
||||
userRestrictionId: fileRestriction('generation', 'Upheld'),
|
||||
status: UserRestrictionStatus.Overturned,
|
||||
moderatorId: MOD_ID,
|
||||
}),
|
||||
400,
|
||||
'Restriction has already been resolved',
|
||||
],
|
||||
];
|
||||
|
||||
for (const [label, call, expectedStatus, expectedReason] of cases) {
|
||||
// `fileRestriction` hardcodes id 1, so stacked rows would all answer to the same lookup and
|
||||
// every case after the first would rule on the previous case's row.
|
||||
store.restrictions.length = 0;
|
||||
|
||||
const { status, body } = await throughRest(call);
|
||||
expect(status, label).toBe(expectedStatus);
|
||||
|
||||
const reason = restErrorReason(body, status);
|
||||
// The discriminating assertion: reading `error` alone returns null here, and null is what
|
||||
// collapses the operator's message back to "<label> returned <status>."
|
||||
expect(reason, label).not.toBeNull();
|
||||
expect(reason, label).toContain(expectedReason);
|
||||
}
|
||||
|
||||
// Positive control on the reader itself — it must be capable of returning null, or the three
|
||||
// `not.toBeNull()` assertions above would hold against a function that always answers.
|
||||
expect(restErrorReason({ nothingReadable: true }, 400)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* An INVARIANT GUARD, not regression coverage — it passes against pre-change code too. Recorded
|
||||
* because it is the reason the service-facing overturn was never the reachable half of this hazard,
|
||||
* and a later "simplification" that drops the predicate would make it one.
|
||||
*/
|
||||
it('overturnPendingReviewMute cannot reach a non-generation row at all', async () => {
|
||||
store.users.set(USER_ID, makeUser(USER_ID, { muted: true }));
|
||||
fileRestriction('bot-account');
|
||||
|
||||
const result = await overturnPendingReviewMute({ userId: USER_ID, moderatorId: MOD_ID });
|
||||
|
||||
expect(result).toEqual({ unmuted: false, skipped: 'no-pending-restriction' });
|
||||
expect(store.restrictions[0].status).toBe('Pending');
|
||||
});
|
||||
|
||||
describe('the wired-for list itself', () => {
|
||||
it('is a subset of the types that can be filed', () => {
|
||||
// A verdict path for a type nothing can file is dead code; the reverse — a filed type with no
|
||||
// verdict path — is the deliberate state this whole guard exists for.
|
||||
for (const type of RULINGS_WIRED_FOR) expect(USER_RESTRICTION_TYPES).toContain(type);
|
||||
});
|
||||
|
||||
it('names generation and refuses everything else', () => {
|
||||
expect([...RULINGS_WIRED_FOR]).toEqual(['generation']);
|
||||
expect(unwiredRulingReason('generation')).toBeNull();
|
||||
for (const type of USER_RESTRICTION_TYPES.filter((t) => !RULINGS_WIRED_FOR.includes(t)))
|
||||
expect(unwiredRulingReason(type)).toContain(`"${type}"`);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/mod/mute-user-pending-review', () => {
|
||||
|
||||
+22
@@ -0,0 +1,22 @@
|
||||
// A fixture for `moderator-restriction-vocabulary.test.ts`, NOT a copy of anything shipped.
|
||||
//
|
||||
// 🔴 The MEASURED walk-through from the #4609 round-2 audit. A `]` inside a trailing comment ends
|
||||
// the old text parser's `[^\]]*` capture, so it read only the first entry of each list and reported
|
||||
// the two apps as agreeing while they did not. Nothing here is hostile — a comment naming an index
|
||||
// is ordinary prose.
|
||||
export const RESTRICTION_TYPES = [
|
||||
'generation', // matches RESTRICTION_TYPES[0] in the main app
|
||||
'bot-account',
|
||||
'spam-account',
|
||||
] as const;
|
||||
|
||||
export const RULINGS_WIRED_FOR: readonly string[] = [
|
||||
'generation', // the only one the main app agrees about — RULINGS_WIRED_FOR[0]
|
||||
'bot-account',
|
||||
];
|
||||
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
return (RULINGS_WIRED_FOR as readonly string[]).includes(type)
|
||||
? null
|
||||
: `Rulings are not yet available for "${type}" restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.`;
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
// A fixture for `moderator-restriction-vocabulary.test.ts`, NOT a copy of anything shipped.
|
||||
//
|
||||
// 🔴 The case NO text parse can reach, at any regex width: neither list is written as a literal at
|
||||
// its own declaration, and the refusal message is assembled rather than templated. A reader that
|
||||
// EXECUTES the module sees the real values; a reader that reads the file as text sees
|
||||
// `'generation', ...RULINGS_ADDED_LATER` and a `return` with no backtick in it.
|
||||
//
|
||||
// 🔴 "Assembled at runtime" here means ASSEMBLED FROM CONSTANTS DECLARED IN THIS FILE, which is the
|
||||
// same value in every environment — that is what the execute reader is entitled to certify, and all
|
||||
// this fixture sanctions. It does NOT sanction a value read from the ENVIRONMENT
|
||||
// (`import.meta.env`, `process.env`): the reader executes the module in the main app's test process,
|
||||
// so such a value is resolved under Vitest and never under the moderator app's production build.
|
||||
// That shape is refused by name — see `assertEnvironmentIndependent` in the harness.
|
||||
const RULINGS_ADDED_LATER = ['bot-account'];
|
||||
const REFUSAL_TAIL =
|
||||
' restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.';
|
||||
|
||||
export const RESTRICTION_TYPES = ['generation', ...RULINGS_ADDED_LATER, 'spam-account'] as const;
|
||||
|
||||
export const RULINGS_WIRED_FOR: readonly string[] = ['generation', ...RULINGS_ADDED_LATER];
|
||||
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
if ((RULINGS_WIRED_FOR as readonly string[]).includes(type)) return null;
|
||||
return 'Rulings are not yet available for "' + type + '"' + REFUSAL_TAIL;
|
||||
}
|
||||
@@ -0,0 +1,13 @@
|
||||
// A fixture for `moderator-restriction-vocabulary.test.ts`, NOT a copy of anything shipped.
|
||||
//
|
||||
// Double-quoted entries. The old text parser extracted `/'([^']+)'/g` only, so a file Prettier had
|
||||
// been configured to double-quote read as an EMPTY list on both counts.
|
||||
export const RESTRICTION_TYPES = ["generation", "bot-account", "spam-account"] as const;
|
||||
|
||||
export const RULINGS_WIRED_FOR: readonly string[] = ["generation", "bot-account"];
|
||||
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
return (RULINGS_WIRED_FOR as readonly string[]).includes(type)
|
||||
? null
|
||||
: `Rulings are not yet available for "${type}" restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.`;
|
||||
}
|
||||
@@ -0,0 +1,21 @@
|
||||
// A fixture for `moderator-restriction-vocabulary.test.ts`, NOT a copy of anything shipped.
|
||||
//
|
||||
// The vocabulary a moderator app WOULD carry if both lists had drifted, written across several
|
||||
// lines. Both lists diverge from the main app's: a third filed type, and a verdict path claimed for
|
||||
// `bot-account` that the main app refuses.
|
||||
export const RESTRICTION_TYPES = [
|
||||
'generation',
|
||||
'bot-account',
|
||||
'spam-account',
|
||||
] as const;
|
||||
|
||||
export const RULINGS_WIRED_FOR: readonly string[] = [
|
||||
'generation',
|
||||
'bot-account',
|
||||
];
|
||||
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
return (RULINGS_WIRED_FOR as readonly string[]).includes(type)
|
||||
? null
|
||||
: `Rulings are not yet available for "${type}" restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.`;
|
||||
}
|
||||
@@ -0,0 +1,13 @@
|
||||
// A fixture for `moderator-restriction-vocabulary.test.ts`, NOT a copy of anything shipped.
|
||||
//
|
||||
// A trailing comma after the last entry — what Prettier writes the moment either list grows past
|
||||
// the print width.
|
||||
export const RESTRICTION_TYPES = ['generation', 'bot-account', 'spam-account',] as const;
|
||||
|
||||
export const RULINGS_WIRED_FOR: readonly string[] = ['generation', 'bot-account',];
|
||||
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
return (RULINGS_WIRED_FOR as readonly string[]).includes(type)
|
||||
? null
|
||||
: `Rulings are not yet available for "${type}" restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.`;
|
||||
}
|
||||
@@ -0,0 +1,215 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
/**
|
||||
* How the main app's suite reads the moderator app's restriction vocabulary: it **imports and
|
||||
* executes** the module and reads the resulting VALUES — plus one TEXT assertion, because the two
|
||||
* mechanisms are blind to different things (see `assertEnvironmentIndependent` below).
|
||||
*
|
||||
* 🔴 It used to parse the file as TEXT with a pair of regexes, and that was walkable. `[^\]]*`
|
||||
* stops at the FIRST `]` after the `=`, so a `]` anywhere inside the literal — in a trailing
|
||||
* comment, in an index expression — truncated the capture, and the extractor only ever read
|
||||
* SINGLE-quoted strings, so a double-quoted entry vanished. Measured on #4609: writing the
|
||||
* moderator list as
|
||||
*
|
||||
* export const RULINGS_WIRED_FOR: readonly RestrictionType[] = [
|
||||
* 'generation', // matches RESTRICTION_TYPES[0]
|
||||
* 'bot-account',
|
||||
* ];
|
||||
*
|
||||
* left the seam suite at 8 passed / 0 failed while the two lists genuinely disagreed. The
|
||||
* `length > 0` positive control could not see it either — the first entry survives the truncation.
|
||||
*
|
||||
* 🔴 Widening the regex would only move the goalposts: a guard that pins source text by PATTERN is
|
||||
* walkable by rewriting the text, and the rewrite that walks it is not a hostile act — it is
|
||||
* someone running Prettier or adding a comment. Executing the module removes the whole class:
|
||||
* formatting, quote style, trailing commas, comments, `as const`, a spread, a value assembled at
|
||||
* runtime and a re-export all produce the same values, because they ARE the same values.
|
||||
*
|
||||
* Why this is possible at all: `apps/moderator/src/lib/restriction-types.ts` has **no imports**.
|
||||
* That is what lets a main-app Vitest project load it despite the moderator app being a separate
|
||||
* SvelteKit build with its own `$lib` aliasing. If someone gives that file a `$lib/…` import this
|
||||
* stops resolving — and it stops LOUDLY, with a module-not-found naming the file, which is the
|
||||
* correct outcome: the two apps would then no longer share a plain-data vocabulary module and the
|
||||
* mirror needs re-deciding rather than silently relaxing.
|
||||
*
|
||||
* 🔴 CROSS-APP BUILD COUPLING, and it lands on the MAIN app's suite. Resolving that path makes Vite
|
||||
* load `apps/moderator/tsconfig.json`, which extends the generated, gitignored
|
||||
* `apps/moderator/.svelte-kit/tsconfig.json` — so without `svelte-kit sync` having been run in
|
||||
* `apps/moderator`, this file and `restriction-type-seam.test.ts` both fail with
|
||||
* `TSConfckParseError: failed to resolve "extends"`, an error naming a tsconfig rather than the
|
||||
* seam. CI is unaffected (`prepare` runs sync); a fresh clone or a fresh worktree is not.
|
||||
*
|
||||
* Not typechecked (`src/**\/__tests__/**` is excluded in tsconfig.json), so the shape is validated
|
||||
* at runtime by `asModeratorVocabulary` below rather than by the compiler.
|
||||
*/
|
||||
|
||||
/** Repo-root-relative, for error messages. */
|
||||
export const MODERATOR_VOCABULARY_FILE = 'apps/moderator/src/lib/restriction-types.ts';
|
||||
|
||||
export const MODERATOR_VOCABULARY_PATH = path.resolve(
|
||||
__dirname,
|
||||
'../../../..',
|
||||
MODERATOR_VOCABULARY_FILE
|
||||
);
|
||||
|
||||
/**
|
||||
* 🔴 The blind spot the execute-based reader has and the text parser it replaced did NOT, so this
|
||||
* check is COMPLEMENTARY to `readModeratorVocabulary` rather than a leftover of it — keep both.
|
||||
*
|
||||
* Executing the module reads its values **in the main app's test process**. An environment-dependent
|
||||
* value is therefore resolved under Vitest's environment, not under the moderator app's production
|
||||
* build. Measured on #4609, writing the list as
|
||||
*
|
||||
* export const RULINGS_WIRED_FOR: readonly RestrictionType[] = import.meta.env.DEV
|
||||
* ? ['generation']
|
||||
* : ['generation', 'bot-account'];
|
||||
*
|
||||
* left the seam and vocabulary suites at 28 passed / 0 failed and the moderator app's own value pin
|
||||
* at 5 passed / 0 failed, while the production build shipped both types — the ban-then-strand hazard
|
||||
* reached with every pinning guard green. The base commit's text parser went RED on that same shape,
|
||||
* so the execute reader is not strictly stronger; it trades a formatting blind spot for a
|
||||
* runtime-environment one.
|
||||
*
|
||||
* 🔴 SCOPE — this refuses the two spellings below and NOT the class. It is a text scan, so it is
|
||||
* walkable by indirection, and that was measured on #4609: an aliased global
|
||||
* (`const P = (globalThis as any).process` then `P?.env`), a computed member access
|
||||
* (`P['env']['NODE_ENV']`), and a regex literal containing `//` placed on the SAME line as the read
|
||||
* — which `withoutComments` below truncates as a line comment — each left the seam and vocabulary
|
||||
* suites at 29 passed / 0 failed while the two apps genuinely disagreed. Every spelling a maintainer
|
||||
* would plausibly reach for IS covered: `import.meta.*` and `process.env.*` are caught here, and
|
||||
* `import { dev } from '$app/environment'` or `$env/*` break loudly as a missing module. Do not read
|
||||
* this as closing the environment-read class.
|
||||
*
|
||||
* 🔴 Note what this means for the "keep this module import-free" precondition: `import.meta.env` and
|
||||
* `process.env` need NO import statement, so import-freedom does not imply environment-independence.
|
||||
* They are two separate constraints and this asserts the second one.
|
||||
*/
|
||||
const ENVIRONMENT_READ = /import\.meta|process\.env/;
|
||||
|
||||
/**
|
||||
* 🔴 Comments are removed before the scan, and that is not a nicety — without it the guard is
|
||||
* matched by its OWN documentation. The module it checks has to be able to say, in prose, which
|
||||
* shapes are refused; a raw-text scan then fires on the sentence forbidding the thing rather than on
|
||||
* the thing, and the only way to keep the suite green is to stop documenting the rule.
|
||||
*
|
||||
* String literals are deliberately KEPT: a `//` inside one must not start a comment, and an
|
||||
* environment read cannot hide inside a string anyway — a string is not executable.
|
||||
*/
|
||||
function withoutComments(sourceText: string): string {
|
||||
let out = '';
|
||||
let i = 0;
|
||||
while (i < sourceText.length) {
|
||||
const c = sourceText[i];
|
||||
const next = sourceText[i + 1];
|
||||
if (c === '/' && next === '/') {
|
||||
while (i < sourceText.length && sourceText[i] !== '\n') i++;
|
||||
continue;
|
||||
}
|
||||
if (c === '/' && next === '*') {
|
||||
i += 2;
|
||||
while (i < sourceText.length && !(sourceText[i] === '*' && sourceText[i + 1] === '/')) i++;
|
||||
i += 2;
|
||||
continue;
|
||||
}
|
||||
if (c === '"' || c === "'" || c === '`') {
|
||||
const quote = c;
|
||||
out += c;
|
||||
i++;
|
||||
while (i < sourceText.length && sourceText[i] !== quote) {
|
||||
if (sourceText[i] === '\\') {
|
||||
out += sourceText[i];
|
||||
i++;
|
||||
}
|
||||
out += sourceText[i];
|
||||
i++;
|
||||
}
|
||||
out += sourceText[i] ?? '';
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
out += c;
|
||||
i++;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* 🔴 Comments are stripped before the scan, string literals are NOT — deliberately, since a string
|
||||
* can be interpolated into a read. The cost is that the scanned module may not MENTION these tokens
|
||||
* in a string either: `export const HINT = 'never read process.env here'` trips this guard. That
|
||||
* fails SAFE — red with this message, never silently green — but it is a real constraint on the
|
||||
* module, and a mention in a COMMENT is the supported way to write one.
|
||||
*/
|
||||
export function assertEnvironmentIndependent(sourceText: string, source: string): void {
|
||||
const found = ENVIRONMENT_READ.exec(withoutComments(sourceText));
|
||||
if (found)
|
||||
throw new Error(
|
||||
`${source} reads \`${found[0]}\`. The vocabulary must be the SAME VALUES in every environment: this guard executes the module in the main app's test process, so an environment-conditional list is read under Vitest and never under the moderator app's production build — the two apps could then ship different lists with every guard green. Write the list as constants, not as a branch on the environment.`
|
||||
);
|
||||
}
|
||||
|
||||
export type ModeratorRestrictionVocabulary = {
|
||||
/** The moderator app's copy of the types that can be FILED and reviewed. */
|
||||
restrictionTypes: readonly string[];
|
||||
/** The moderator app's copy of the types a VERDICT may be handed to. */
|
||||
rulingsWiredFor: readonly string[];
|
||||
/** The moderator app's refusal message, executed rather than read out of its template literal. */
|
||||
unwiredRulingReason: (type: string) => string | null;
|
||||
};
|
||||
|
||||
function stringArray(value: unknown, name: string, source: string): readonly string[] {
|
||||
if (!Array.isArray(value) || value.some((v) => typeof v !== 'string'))
|
||||
throw new Error(
|
||||
`\`${name}\` in ${source} is not an array of strings. If the vocabulary moved or changed shape, update this guard — do not delete it.`
|
||||
);
|
||||
// A non-empty list is the positive control the old text parse needed a separate test for: an
|
||||
// empty one would make every comparison below compare two empty-ish things and pass.
|
||||
if (value.length === 0)
|
||||
throw new Error(
|
||||
`\`${name}\` in ${source} is empty. That cannot be right, and an empty list would make every comparison against it vacuous.`
|
||||
);
|
||||
return value as readonly string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the shape of an imported vocabulary module and projects it. Throws — loudly, naming the
|
||||
* file — rather than returning something empty, so a renamed or removed export reports as a broken
|
||||
* guard instead of silently matching nothing.
|
||||
*/
|
||||
export function asModeratorVocabulary(
|
||||
mod: unknown,
|
||||
source: string
|
||||
): ModeratorRestrictionVocabulary {
|
||||
const m = (mod ?? {}) as Record<string, unknown>;
|
||||
// Order matters only for the error you get back: the lists are checked first so a module that is
|
||||
// missing everything reports the first thing it is missing rather than the last.
|
||||
const restrictionTypes = stringArray(m.RESTRICTION_TYPES, 'RESTRICTION_TYPES', source);
|
||||
const rulingsWiredFor = stringArray(m.RULINGS_WIRED_FOR, 'RULINGS_WIRED_FOR', source);
|
||||
if (typeof m.unwiredRulingReason !== 'function')
|
||||
throw new Error(
|
||||
`\`unwiredRulingReason\` in ${source} is not a function. If it moved, update this guard — do not delete it.`
|
||||
);
|
||||
return {
|
||||
restrictionTypes,
|
||||
rulingsWiredFor,
|
||||
unwiredRulingReason: m.unwiredRulingReason as (type: string) => string | null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Import-and-execute. `file` is defaulted to the real moderator module; the fixture suite passes
|
||||
* its own copies so the reader itself is under test, not only the pairing it happens to read today.
|
||||
*/
|
||||
export async function readModeratorVocabulary(
|
||||
file: string = MODERATOR_VOCABULARY_PATH
|
||||
): Promise<ModeratorRestrictionVocabulary> {
|
||||
const source = file === MODERATOR_VOCABULARY_PATH ? MODERATOR_VOCABULARY_FILE : file;
|
||||
// The TEXT half, run BEFORE the import so the failure names the constraint rather than reporting a
|
||||
// list that happens to be correct in this process. See `assertEnvironmentIndependent`.
|
||||
assertEnvironmentIndependent(fs.readFileSync(file, 'utf-8'), source);
|
||||
// `@vite-ignore` because the path is computed: this reader is deliberately usable against a
|
||||
// fixture, which is the only way to test that it sees a divergence at all.
|
||||
const mod = await import(/* @vite-ignore */ file);
|
||||
return asModeratorVocabulary(mod, source);
|
||||
}
|
||||
@@ -0,0 +1,189 @@
|
||||
import path from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
asModeratorVocabulary,
|
||||
assertEnvironmentIndependent,
|
||||
MODERATOR_VOCABULARY_FILE,
|
||||
readModeratorVocabulary,
|
||||
} from './moderator-restriction-vocabulary.harness';
|
||||
import {
|
||||
RULINGS_WIRED_FOR,
|
||||
USER_RESTRICTION_TYPES,
|
||||
unwiredRulingReason,
|
||||
} from '~/server/services/user-restriction.service';
|
||||
|
||||
/**
|
||||
* The READER behind `restriction-type-seam.test.ts`, tested against fixtures rather than only
|
||||
* against the one pairing it happens to read today.
|
||||
*
|
||||
* 🔴 Why this file exists. The seam guard used to read the moderator app's vocabulary as TEXT, with
|
||||
* `/…= \[([^\]]*)\]/` plus a single-quoted-string extractor. Measured on #4609: with the moderator
|
||||
* list written across several lines and a comment naming an index, the seam suite reported
|
||||
* **8 passed / 0 failed** while the two apps genuinely disagreed about which restriction types a
|
||||
* verdict may be handed to — the `]` in the comment truncated the capture, and the first entry
|
||||
* survived the truncation, so even the `length > 0` positive control stayed green.
|
||||
*
|
||||
* That divergence is not cosmetic. If the moderator app believes `bot-account` is rulable, its
|
||||
* audit-queue `ban` action bans the account and posts the ruling AFTERWARDS — a ruling the main app
|
||||
* refuses — leaving a banned account with a Pending row nobody can close.
|
||||
*
|
||||
* 🔴 The fix is not a wider regex. A guard that pins source text by PATTERN is walkable by
|
||||
* rewriting the text, and the rewrites that walk it are ordinary: a Prettier reflow, a comment, a
|
||||
* quote-style change. The reader now IMPORTS AND EXECUTES the module, so formatting cannot be the
|
||||
* difference between agreeing and disagreeing. Each fixture below is a formatting that the text
|
||||
* parser either misread or could not have read at all; every one of them carries a REAL divergence,
|
||||
* so a reader that could not see it would report agreement.
|
||||
*/
|
||||
|
||||
const fixture = (name: string) =>
|
||||
path.resolve(__dirname, '__fixtures__/moderator-vocabulary', `${name}.ts`);
|
||||
|
||||
/**
|
||||
* Every fixture declares the SAME two lists, in a different shape:
|
||||
* RESTRICTION_TYPES = generation, bot-account, spam-account (main app: generation, bot-account)
|
||||
* RULINGS_WIRED_FOR = generation, bot-account (main app: generation)
|
||||
* so one expectation covers all of them and a reader that silently returns something else fails.
|
||||
*/
|
||||
const FIXTURE_TYPES = ['generation', 'bot-account', 'spam-account'];
|
||||
const FIXTURE_WIRED = ['generation', 'bot-account'];
|
||||
|
||||
const FIXTURES: readonly (readonly [string, string])[] = [
|
||||
['a multi-line array', 'multi-line-array'],
|
||||
['a comment containing a closing bracket', 'comment-with-bracket'],
|
||||
['double-quoted entries', 'double-quoted'],
|
||||
['a trailing comma', 'trailing-comma'],
|
||||
['values assembled at runtime', 'computed'],
|
||||
];
|
||||
|
||||
describe('moderator restriction vocabulary — the reader the seam guard uses', () => {
|
||||
it('has fixtures that genuinely disagree with the main app, or the cases below prove nothing', () => {
|
||||
// The control that makes every fixture case non-vacuous. If the main app ever widened to match
|
||||
// these, the fixtures would stop being divergences and would pass against a reader that could
|
||||
// not see anything at all.
|
||||
expect([...FIXTURE_WIRED].sort()).not.toEqual([...RULINGS_WIRED_FOR].sort());
|
||||
expect([...FIXTURE_TYPES].sort()).not.toEqual([...USER_RESTRICTION_TYPES].sort());
|
||||
});
|
||||
|
||||
it.each(FIXTURES)('reads both lists through %s', async (_label, name) => {
|
||||
const vocabulary = await readModeratorVocabulary(fixture(name));
|
||||
|
||||
expect([...vocabulary.restrictionTypes]).toEqual(FIXTURE_TYPES);
|
||||
expect([...vocabulary.rulingsWiredFor]).toEqual(FIXTURE_WIRED);
|
||||
});
|
||||
|
||||
/**
|
||||
* The refusal SENTENCE, which the text parser read out of a template literal with a third regex
|
||||
* and which is now simply called. Pinned separately from the list because the two fail
|
||||
* differently: a drifted list is the ban-then-strand hazard above, a drifted sentence is two
|
||||
* surfaces explaining one refusal in two ways.
|
||||
*/
|
||||
it.each(FIXTURES)('reads the refusal message by executing it, through %s', async (_l, name) => {
|
||||
const vocabulary = await readModeratorVocabulary(fixture(name));
|
||||
|
||||
// The fixture claims a verdict path for `bot-account` that the main app refuses — the exact
|
||||
// divergence, expressed as behaviour rather than as text.
|
||||
expect(vocabulary.unwiredRulingReason('bot-account')).toBeNull();
|
||||
expect(unwiredRulingReason('bot-account')).not.toBeNull();
|
||||
|
||||
// …and where the two DO agree, they agree word for word, so this is not merely detecting that
|
||||
// something changed.
|
||||
expect(vocabulary.unwiredRulingReason('generation')).toBeNull();
|
||||
expect(vocabulary.unwiredRulingReason('spam-account')).toBe(
|
||||
unwiredRulingReason('spam-account')
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The half executing the module GAVE UP, kept alongside it rather than instead of it. The
|
||||
* execute reader resolves the vocabulary in the MAIN APP'S TEST PROCESS, so a list branching on the
|
||||
* environment is read under Vitest and never under the moderator app's production build. Measured
|
||||
* on #4609: with `RULINGS_WIRED_FOR = import.meta.env.DEV ? ['generation'] : ['generation',
|
||||
* 'bot-account']`, the seam and vocabulary suites reported 28 passed / 0 failed and the moderator's
|
||||
* own value pin 5 passed / 0 failed, while the shipped build carried both types. The base commit's
|
||||
* TEXT parser went red on that same module — so the reader that replaced it traded one blind spot
|
||||
* for another, and this is the trade being paid back.
|
||||
*/
|
||||
it('refuses a vocabulary whose values are read from the environment', () => {
|
||||
// Both shapes, because neither needs an import statement and the module's stated precondition is
|
||||
// only about imports.
|
||||
expect(() =>
|
||||
assertEnvironmentIndependent(
|
||||
`export const RULINGS_WIRED_FOR = import.meta.env.DEV ? ['generation'] : ['generation', 'bot-account'];`,
|
||||
MODERATOR_VOCABULARY_FILE
|
||||
)
|
||||
).toThrow(/import\.meta/);
|
||||
expect(() =>
|
||||
assertEnvironmentIndependent(
|
||||
`export const RULINGS_WIRED_FOR = process.env.NODE_ENV === 'test' ? ['generation'] : [];`,
|
||||
MODERATOR_VOCABULARY_FILE
|
||||
)
|
||||
).toThrow(/process\.env/);
|
||||
// Negative control: an ordinary literal list must NOT trip it, or the check would be refusing
|
||||
// everything and its red above would say nothing.
|
||||
expect(() =>
|
||||
assertEnvironmentIndependent(
|
||||
`export const RULINGS_WIRED_FOR = ['generation'];`,
|
||||
MODERATOR_VOCABULARY_FILE
|
||||
)
|
||||
).not.toThrow();
|
||||
// 🔴 …and the control that stops this guard being matched by its own documentation. The module
|
||||
// has to be able to NAME the shapes it refuses; a scan over raw text fires on that sentence, and
|
||||
// the only way back to green would be to delete the explanation.
|
||||
expect(() =>
|
||||
assertEnvironmentIndependent(
|
||||
[
|
||||
'/** Do not read `import.meta.env` or `process.env` here — see the harness. */',
|
||||
'// process.env is likewise refused.',
|
||||
`export const RULINGS_WIRED_FOR = ['generation'];`,
|
||||
].join('\n'),
|
||||
MODERATOR_VOCABULARY_FILE
|
||||
)
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('loads the real moderator module when given no path', async () => {
|
||||
// Positive control on the default argument: the fixture cases would all pass against a reader
|
||||
// pointed at nothing real.
|
||||
const vocabulary = await readModeratorVocabulary();
|
||||
|
||||
expect([...vocabulary.restrictionTypes]).toContain('generation');
|
||||
expect(typeof vocabulary.unwiredRulingReason).toBe('function');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The reader must fail LOUDLY, not emptily. A reader that returned `[]` for a module it could
|
||||
* not understand would make the seam guard's equality assertions compare two nearly-empty things —
|
||||
* the reassuring-zero shape the text parser died of.
|
||||
*/
|
||||
describe('moderator restriction vocabulary — refuses a module it cannot read', () => {
|
||||
const ok = {
|
||||
RESTRICTION_TYPES: ['generation'],
|
||||
RULINGS_WIRED_FOR: ['generation'],
|
||||
unwiredRulingReason: () => null,
|
||||
};
|
||||
|
||||
it('accepts a well-formed module, so the refusals below are specific', () => {
|
||||
expect(asModeratorVocabulary(ok, MODERATOR_VOCABULARY_FILE).restrictionTypes).toEqual([
|
||||
'generation',
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['a renamed type list', { ...ok, RESTRICTION_TYPES: undefined }, 'RESTRICTION_TYPES'],
|
||||
['a renamed wired-for list', { ...ok, RULINGS_WIRED_FOR: undefined }, 'RULINGS_WIRED_FOR'],
|
||||
['an empty type list', { ...ok, RESTRICTION_TYPES: [] }, 'RESTRICTION_TYPES'],
|
||||
['an empty wired-for list', { ...ok, RULINGS_WIRED_FOR: [] }, 'RULINGS_WIRED_FOR'],
|
||||
['a list holding a non-string', { ...ok, RULINGS_WIRED_FOR: ['a', 1] }, 'RULINGS_WIRED_FOR'],
|
||||
[
|
||||
'a missing refusal function',
|
||||
{ ...ok, unwiredRulingReason: undefined },
|
||||
'unwiredRulingReason',
|
||||
],
|
||||
['nothing at all', undefined, 'RESTRICTION_TYPES'],
|
||||
])('throws on %s, naming what it could not read', (_label, mod, named) => {
|
||||
expect(() => asModeratorVocabulary(mod, MODERATOR_VOCABULARY_FILE)).toThrow(
|
||||
new RegExp(`\`${named}\``)
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,148 @@
|
||||
import { beforeAll, describe, expect, it } from 'vitest';
|
||||
import {
|
||||
readModeratorVocabulary,
|
||||
type ModeratorRestrictionVocabulary,
|
||||
} from './moderator-restriction-vocabulary.harness';
|
||||
import {
|
||||
PENDING_REVIEW_MUTE_NOTIFICATION,
|
||||
RULINGS_WIRED_FOR,
|
||||
USER_RESTRICTION_TYPES,
|
||||
unwiredRulingReason,
|
||||
} from '~/server/services/user-restriction.service';
|
||||
|
||||
/**
|
||||
* The seam between the three things that have to agree about a restriction type, none of which imports
|
||||
* the others in production (this file does, deliberately — see below):
|
||||
*
|
||||
* 1. the main app, which FILES restrictions of a type;
|
||||
* 2. the moderator app, which is the only place one can be REVIEWED;
|
||||
* 3. the notification registry, which decides whether the user is told anything intelligible.
|
||||
*
|
||||
* 🔴 Each of the three is separately covered by a suite that loads only its own surface, and that is
|
||||
* exactly why this file exists: a type can be filed by an app whose queue view cannot list it, or
|
||||
* mapped to a notification that nothing renders, without a single one of those suites going red. The
|
||||
* defect lives in the seam nobody owns.
|
||||
*/
|
||||
|
||||
/**
|
||||
* 🔴 The moderator app's vocabulary is IMPORTED AND EXECUTED, not parsed. It used to be read as
|
||||
* TEXT, and that guard passed green over a real divergence: `/…= \[([^\]]*)\]/` stops at the first
|
||||
* `]` after the `=`, so a comment naming an index truncated the capture, and the extractor read only
|
||||
* single-quoted strings, so a double-quoted entry vanished. Measured on #4609 — this file reported
|
||||
* **8 passed / 0 failed** with the two lists genuinely disagreeing.
|
||||
*
|
||||
* A wider regex would not have fixed it. A guard that pins source text by PATTERN is walkable by
|
||||
* reformatting the text, and the reformattings that walk it are ordinary. Executing the module makes
|
||||
* formatting irrelevant by construction.
|
||||
*
|
||||
* That the import resolves at all rests on one fact: `apps/moderator/src/lib/restriction-types.ts`
|
||||
* has no imports of its own, so the main app's Vitest project can load it even though the moderator
|
||||
* app is a separate SvelteKit build with its own `$lib` aliasing. Give that file a `$lib/…` import
|
||||
* and this fails loudly, naming the module — which is the right outcome, not something to work
|
||||
* around.
|
||||
*
|
||||
* The reader, its shape validation and the fixtures that prove it sees a divergence live in
|
||||
* `src/server/services/__tests__/moderator-restriction-vocabulary.harness.ts` and
|
||||
* `src/server/services/__tests__/moderator-restriction-vocabulary.test.ts`.
|
||||
*/
|
||||
let moderator: ModeratorRestrictionVocabulary;
|
||||
|
||||
beforeAll(async () => {
|
||||
moderator = await readModeratorVocabulary();
|
||||
});
|
||||
|
||||
describe('restriction type — main app ⇄ moderator app', () => {
|
||||
// A positive control on the reader. It cannot come back empty — `readModeratorVocabulary` throws
|
||||
// on an empty or unreadable list rather than returning one — but this pins the fact rather than
|
||||
// trusting a helper in another file to keep doing it.
|
||||
it('reads a non-empty type list out of the moderator app', () => {
|
||||
expect(moderator.restrictionTypes.length).toBeGreaterThan(0);
|
||||
expect(moderator.restrictionTypes).toContain('generation');
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 Fails when the sets DIFFER IN EITHER DIRECTION, which is the point — the two failure modes are
|
||||
* opposite and both silent:
|
||||
*
|
||||
* - a type in the main app but not the moderator app files cases into a queue with no view, so a
|
||||
* detector's findings are muted accounts nobody can ever see or clear;
|
||||
* - a type in the moderator app but not the main app is a queue tab that can only ever be empty.
|
||||
*/
|
||||
it('files exactly the types the moderator queue can show', () => {
|
||||
expect([...moderator.restrictionTypes].sort()).toEqual([...USER_RESTRICTION_TYPES].sort());
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The third thing that has to agree, added after the audit on #4609: WHICH types a verdict may be
|
||||
* handed to. That is enforced in the main app, inside `resolveUserRestriction`, because five callers
|
||||
* reach it and a guard replicated per route is wrong at all but one of them. The moderator app holds a
|
||||
* copy anyway, and needs to — a list read forward is what lets a form that cannot possibly succeed be
|
||||
* DISABLED rather than merely rejected, and what lets the audit queue refuse a ban BEFORE it bans
|
||||
* (that action bans and then rules, so a late refusal strands a Pending row on a banned account).
|
||||
*
|
||||
* Two separate builds with no runtime import path between them, so the copy is pinned here rather than
|
||||
* left to drift. A moderator app that thought `bot-account` was rulable would render live Uphold and
|
||||
* Ban buttons whose only possible outcome is a rejected call.
|
||||
*/
|
||||
describe('restriction type — ruling scope ⇄ moderator app', () => {
|
||||
it('reads a non-empty wired-for list out of the moderator app', () => {
|
||||
expect(moderator.rulingsWiredFor.length).toBeGreaterThan(0);
|
||||
expect(moderator.rulingsWiredFor).toContain('generation');
|
||||
});
|
||||
|
||||
it('agrees with the main app about which types a verdict can be handed to', () => {
|
||||
expect([...moderator.rulingsWiredFor].sort()).toEqual([...RULINGS_WIRED_FOR].sort());
|
||||
});
|
||||
|
||||
it('refuses the same types on both sides, word for word', () => {
|
||||
// Every type that can be FILED, so a type added to the vocabulary without a verdict path is
|
||||
// covered here the day it is added rather than the day someone remembers this file. The
|
||||
// moderator side is CALLED, not read out of its template literal — so a message assembled from
|
||||
// constants, or moved behind a helper, is compared on what it produces.
|
||||
for (const type of USER_RESTRICTION_TYPES)
|
||||
expect(moderator.unwiredRulingReason(type)).toEqual(unwiredRulingReason(type));
|
||||
|
||||
// The whole comparison above is vacuous if no type is currently refused — assert one is.
|
||||
expect(USER_RESTRICTION_TYPES.some((t) => unwiredRulingReason(t) !== null)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('restriction type — notification mapping', () => {
|
||||
it('maps every restriction type, so a new one cannot default into someone else’s message', () => {
|
||||
expect(Object.keys(PENDING_REVIEW_MUTE_NOTIFICATION).sort()).toEqual(
|
||||
[...USER_RESTRICTION_TYPES].sort()
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The guard that makes `null` safe to rely on. `createNotification` validates `type` against
|
||||
* nothing — it is `z.string()` at the schema and `text` at the table, and the fan-out worker inserts
|
||||
* it verbatim — so an unregistered type is PERSISTED and increments the user's unread badge, while
|
||||
* the bell dropdown drops it at render because no processor can build a message for it. The result is
|
||||
* a phantom unread count with no click target, clearable only by "mark all read".
|
||||
*
|
||||
* So: any type this map names must be a registered processor key. Adding a mapping without adding
|
||||
* the processor fails here rather than shipping a ghost notification.
|
||||
*/
|
||||
it('names only registered notification processors', async () => {
|
||||
const { notificationProcessors } = await import('~/server/notifications/utils.notifications');
|
||||
const registered = Object.keys(notificationProcessors);
|
||||
|
||||
// Positive control: the registry actually loaded and holds the key the generation path uses. A
|
||||
// `registered` that came back empty would make the loop below vacuously true.
|
||||
expect(registered).toContain('generation-muted');
|
||||
|
||||
const mapped = Object.values(PENDING_REVIEW_MUTE_NOTIFICATION).filter(
|
||||
(v): v is string => v !== null
|
||||
);
|
||||
expect(mapped.length).toBeGreaterThan(0);
|
||||
for (const type of mapped) expect(registered).toContain(type);
|
||||
});
|
||||
|
||||
it('keeps generation on the message written for it', () => {
|
||||
// Pinned by value: silently repointing generation at another processor would change what every
|
||||
// muted user is told, and no other test in this repo reads the mapping.
|
||||
expect(PENDING_REVIEW_MUTE_NOTIFICATION.generation).toBe('generation-muted');
|
||||
});
|
||||
});
|
||||
@@ -10,13 +10,23 @@ import { updateUserById } from '~/server/services/user.service';
|
||||
import { clearedMuteFields } from '~/server/services/mute-provenance';
|
||||
import { dbRead } from '~/server/db/client';
|
||||
import type { UserMeta } from '~/server/schema/user.schema';
|
||||
import { PROTECTED_USER_IDS } from '~/server/services/user-restriction.service';
|
||||
import {
|
||||
PROTECTED_USER_IDS,
|
||||
unwiredRulingReason,
|
||||
} from '~/server/services/user-restriction.service';
|
||||
import { throwBadRequestError, throwNotFoundError } from '~/server/utils/errorHandling';
|
||||
import { UserRestrictionStatus } from '~/shared/utils/prisma/enums';
|
||||
|
||||
/**
|
||||
* Uphold or overturn a generation restriction. The single write path for a
|
||||
* verdict — the moderator router and the service-facing overturn endpoint both
|
||||
* go through here so the membership and violation-count side effects can't drift.
|
||||
*
|
||||
* 🔴 Being the single write path is also why the type refusal lives here rather than at the routes.
|
||||
* Everything below this line is generation-shaped — the notification types, the update source, the
|
||||
* email wording, and `resetProhibitedRequestCount`, which wipes the account's real prompt-violation
|
||||
* counter. Five callers reach it (the tRPC router, `/api/mod/restriction/resolve`, and
|
||||
* `overturnPendingReviewMute`), and only one of them used to check. See `unwiredRulingReason`.
|
||||
*/
|
||||
export async function resolveUserRestriction({
|
||||
userRestrictionId,
|
||||
@@ -35,13 +45,28 @@ export async function resolveUserRestriction({
|
||||
id: true,
|
||||
userId: true,
|
||||
status: true,
|
||||
// Read back rather than assumed: callers address the row by primary key, so none of them can
|
||||
// tell what type it is, and the refusal below is the only thing that looks.
|
||||
type: true,
|
||||
user: { select: { email: true, username: true } },
|
||||
},
|
||||
});
|
||||
|
||||
if (!restriction) throw new Error('Restriction record not found');
|
||||
// 🔴 TRPCErrors, not bare `Error`s, and that is the difference between a moderator reading the
|
||||
// reason and reading nothing. Both ruling surfaces post through `/api/mod/restriction/resolve`,
|
||||
// whose `defineModeratorEndpoint` wrapper hands a thrown value to `handleEndpointError`. A
|
||||
// non-TRPCError falls to its catch-all branch and reaches the wire as **500 "An unexpected error
|
||||
// occurred"** — the retool panel then renders "Restriction ruling: An unexpected error occurred."
|
||||
// and the whole point of the refusal is destroyed. A TRPCError keeps its status AND its message.
|
||||
//
|
||||
// All three are 4xx: each is a fact about the request, none is a server fault.
|
||||
if (!restriction) throw throwNotFoundError('Restriction record not found');
|
||||
// Checked BEFORE the already-resolved test and before any write: a row this path cannot rule on is
|
||||
// not a row whose status is worth arguing about.
|
||||
const unwired = unwiredRulingReason(restriction.type);
|
||||
if (unwired) throw throwBadRequestError(unwired);
|
||||
if (restriction.status !== UserRestrictionStatus.Pending)
|
||||
throw new Error('Restriction has already been resolved');
|
||||
throw throwBadRequestError('Restriction has already been resolved');
|
||||
|
||||
await dbWrite.userRestriction.update({
|
||||
where: { id: userRestrictionId },
|
||||
|
||||
@@ -16,6 +16,78 @@ export const PROTECTED_USER_IDS = new Set<number>([
|
||||
constants.system.officialUserId,
|
||||
]);
|
||||
|
||||
/**
|
||||
* The kinds of review that file into the moderator mute queue.
|
||||
*
|
||||
* `UserRestriction.type` is a free-text column carrying a `[type, status]` index, so a new kind costs
|
||||
* no migration — but it does need a queue view that shows it, which is why this is an enumerated union
|
||||
* rather than a bare `string`. A typo would otherwise file a row into a type nothing lists.
|
||||
*
|
||||
* Mirrored for the moderator app in `apps/moderator/src/lib/server/user-restriction.service.ts`; the
|
||||
* two lists are pinned to each other by `src/server/services/__tests__/restriction-type-seam.test.ts`.
|
||||
*/
|
||||
export const USER_RESTRICTION_TYPES = ['generation', 'bot-account'] as const;
|
||||
export type UserRestrictionType = (typeof USER_RESTRICTION_TYPES)[number];
|
||||
|
||||
export const DEFAULT_USER_RESTRICTION_TYPE: UserRestrictionType = 'generation';
|
||||
|
||||
/**
|
||||
* The notification a pending-review mute sends, per restriction type — `null` meaning "say nothing".
|
||||
*
|
||||
* 🔴 An OPT-IN map, and the `null` is the safe half rather than a gap. Two things make it the right
|
||||
* shape:
|
||||
*
|
||||
* 1. `createNotification` does not validate `type` against anything. It is `z.string()` at the schema,
|
||||
* `text` at both tables, and the fan-out worker inserts it verbatim — so an unregistered type is
|
||||
* persisted and *increments the user's unread badge*, while the bell dropdown drops it at render
|
||||
* (`getNotificationMessage` returns null for an unknown type and the list `.filter(isDefined)`s it
|
||||
* away). The result is a phantom unread count with no click target, clearable only by "mark all
|
||||
* read". Sending an unregistered type is therefore worse than sending none.
|
||||
* 2. Reusing `generation-muted` for a non-generation mute would tell a user their *generation access*
|
||||
* was restricted for something that has nothing to do with generation.
|
||||
*
|
||||
* So a new type stays silent until someone deliberately (a) adds a processor for it under
|
||||
* `src/server/notifications/` and reaches it from `notificationProcessors`, and (b) names it here. The
|
||||
* seam test asserts every value in this map is a registered processor key, so a mapping added without
|
||||
* the processor fails rather than ships a ghost notification.
|
||||
*/
|
||||
export const PENDING_REVIEW_MUTE_NOTIFICATION: Record<UserRestrictionType, string | null> = {
|
||||
generation: 'generation-muted',
|
||||
'bot-account': null,
|
||||
};
|
||||
|
||||
/**
|
||||
* The restriction types a moderator's verdict can actually be applied to.
|
||||
*
|
||||
* 🔴 Deliberately NARROWER than `USER_RESTRICTION_TYPES`: a type can be *filed and reviewed* long
|
||||
* before anyone builds a verdict path for it. `resolveUserRestriction` is still generation-shaped —
|
||||
* it hardcodes the `generation-restriction-upheld` / `-overturned` notification types, a
|
||||
* `moderator:generationRestriction*` update source and a generation-worded email, and on an overturn
|
||||
* it calls `resetProhibitedRequestCount`, which wipes the account's *prompt*-violation counter. Run
|
||||
* that against a bot-account row and the user is told their generation access was restored over
|
||||
* something that has nothing to do with generation, and a real counter is cleared with it.
|
||||
*
|
||||
* Adding a type here means parameterising that verdict path first.
|
||||
*/
|
||||
export const RULINGS_WIRED_FOR: readonly UserRestrictionType[] = ['generation'];
|
||||
|
||||
/**
|
||||
* Why a verdict may not be handed to a row of this type, or `null` when it may.
|
||||
*
|
||||
* 🔴 Lives HERE, one level below every ruling surface, on purpose. There are five entry points into
|
||||
* `resolveUserRestriction` — the tRPC router, `/api/mod/restriction/resolve` (which is what BOTH
|
||||
* moderator-app ruling surfaces post through: the audit queue and the retool User Lookup panel), and
|
||||
* `overturnPendingReviewMute` — and a guard replicated per route is a predicate open-coded at N sites,
|
||||
* wrong at N−1 of them. The moderator app cannot import this module (separate build, separate
|
||||
* project), so its copy of the list is pinned to this one by
|
||||
* `src/server/services/__tests__/restriction-type-seam.test.ts` rather than left to drift.
|
||||
*/
|
||||
export function unwiredRulingReason(type: string): string | null {
|
||||
return (RULINGS_WIRED_FOR as readonly string[]).includes(type)
|
||||
? null
|
||||
: `Rulings are not yet available for "${type}" restrictions — the verdict path still sends generation-specific notices. This restriction was NOT resolved.`;
|
||||
}
|
||||
|
||||
export type PendingReviewMuteResult =
|
||||
| { muted: true; userRestrictionId: number; deduped: boolean }
|
||||
| { muted: false; skipped: 'protected' | 'moderator' | 'banned' | 'deleted' };
|
||||
@@ -35,16 +107,50 @@ async function bestEffort(name: string, userId: number, fn: () => Promise<unknow
|
||||
* `mutedAt` is deliberately not written. It marks a moderator's uphold, and
|
||||
* `confirm-mutes` cancels the user's memberships off a recent non-null value —
|
||||
* so setting it here would bill-punish an unreviewed account.
|
||||
*
|
||||
* `type` selects which review queue the case is filed into, and defaults to the only one that existed
|
||||
* before it was a parameter. Dedupe is scoped to it: a user already holding an open case of one type
|
||||
* can still be muted under another, because otherwise the first open case would swallow every later
|
||||
* finding of a different kind and the second queue would simply never fill.
|
||||
*/
|
||||
export async function applyPendingReviewMute({
|
||||
userId,
|
||||
triggers,
|
||||
updateSource,
|
||||
type = DEFAULT_USER_RESTRICTION_TYPE,
|
||||
}: {
|
||||
userId: number;
|
||||
triggers: unknown[];
|
||||
updateSource: string;
|
||||
type?: UserRestrictionType;
|
||||
}): Promise<PendingReviewMuteResult> {
|
||||
// 🔴 Runtime, not just TypeScript — and it is worth being exact about why, because the obvious
|
||||
// justification is not true here. NOTHING crosses an HTTP boundary into this parameter today:
|
||||
// neither production caller passes a `type` at all, and the one HTTP route that reaches this
|
||||
// function (`src/pages/api/mod/mute-user-pending-review.ts`) has no `type` key in its zod schema,
|
||||
// so no request body can supply one. Every value arriving here is written by an in-process caller
|
||||
// the compiler can see.
|
||||
//
|
||||
// What the guard is for is the SHAPE OF THE NEXT CALLER. This is the one seam whose entire purpose
|
||||
// is accepting a caller-supplied type, it exists so a detector can file into this queue, and the
|
||||
// obvious way to wire one up is a route that forwards a field off a JSON body — at which point the
|
||||
// compiler's word is worth nothing and the guard is the only thing standing there. It also covers
|
||||
// the callers TypeScript cannot vouch for today: an `as` cast, a value read back from the
|
||||
// free-text `UserRestriction.type` column, or a JS caller.
|
||||
//
|
||||
// The harm it prevents is not a harmless typo: an out-of-vocabulary value MUTES the account, files
|
||||
// a row the queue's `z.enum(RESTRICTION_TYPES).catch(...)` can never select, and — via
|
||||
// `PENDING_REVIEW_MUTE_NOTIFICATION[type]` coming back `undefined` — tells the user nothing. The
|
||||
// result is a silently muted account with no reviewable case anywhere.
|
||||
//
|
||||
// A throw rather than a `skipped` result: the `skipped` union describes facts about the USER that a
|
||||
// caller is expected to handle, and this is a defect in the CALLER. Thrown before any write, so a
|
||||
// rejected call mutes nobody.
|
||||
if (!(USER_RESTRICTION_TYPES as readonly string[]).includes(type))
|
||||
throw new Error(
|
||||
`Unknown user restriction type "${type}". Known types: ${USER_RESTRICTION_TYPES.join(', ')}.`
|
||||
);
|
||||
|
||||
if (PROTECTED_USER_IDS.has(userId)) return { muted: false, skipped: 'protected' };
|
||||
|
||||
// Primary, not the replica: this is a security gate, and replica lag would let
|
||||
@@ -59,7 +165,7 @@ export async function applyPendingReviewMute({
|
||||
if (user.bannedAt) return { muted: false, skipped: 'banned' };
|
||||
|
||||
const existing = await dbWrite.userRestriction.findFirst({
|
||||
where: { userId, type: 'generation', status: UserRestrictionStatus.Pending },
|
||||
where: { userId, type, status: UserRestrictionStatus.Pending },
|
||||
orderBy: { createdAt: 'desc' },
|
||||
select: { id: true },
|
||||
});
|
||||
@@ -79,7 +185,7 @@ export async function applyPendingReviewMute({
|
||||
dbWrite.userRestriction.create({
|
||||
data: {
|
||||
userId,
|
||||
type: 'generation',
|
||||
type,
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
triggers: triggers as any,
|
||||
},
|
||||
@@ -94,15 +200,18 @@ export async function applyPendingReviewMute({
|
||||
await bestEffort('pending-review-mute-refresh-session-failed', userId, () =>
|
||||
refreshSession(userId, { caller: 'moderation' })
|
||||
);
|
||||
await bestEffort('pending-review-mute-notify-failed', userId, () =>
|
||||
createNotification({
|
||||
type: 'generation-muted',
|
||||
key: `generation-muted:${userId}:${userRestrictionId}`,
|
||||
category: NotificationCategory.System,
|
||||
userId,
|
||||
details: {},
|
||||
})
|
||||
);
|
||||
const notificationType = PENDING_REVIEW_MUTE_NOTIFICATION[type];
|
||||
if (notificationType) {
|
||||
await bestEffort('pending-review-mute-notify-failed', userId, () =>
|
||||
createNotification({
|
||||
type: notificationType,
|
||||
key: `${notificationType}:${userId}:${userRestrictionId}`,
|
||||
category: NotificationCategory.System,
|
||||
userId,
|
||||
details: {},
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
return { muted: true, userRestrictionId, deduped };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user