mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Zachary Lowden
ec49115e55
* 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.
37 lines
2.5 KiB
Plaintext
37 lines
2.5 KiB
Plaintext
# apps/creator-studio formats itself, from its own root, with its own Prettier 3 + prettier-plugin-svelte
|
|
# (svelte 5 needs prettier >=3; no prettier-plugin-svelte supports svelte 5 on the root's prettier 2).
|
|
# The two majors format TS differently -- prettier 3 collapses leading-pipe unions that prettier 2 breaks
|
|
# across lines -- so ownership has to be exclusive or the two fight over the same files forever.
|
|
# Run `pnpm -F @civitai/creator-studio-app format` there instead.
|
|
apps/creator-studio/
|
|
|
|
# Generated by `pnpm --filter @civitai/db-schema drift:baseline`, and the ONLY consumer of its
|
|
# format is the diff a reviewer reads when someone accepts new schema drift. Prettier and
|
|
# JSON.stringify(_, null, 2) disagree about it by 246 lines, so with both formatting it a
|
|
# no-op regeneration produced a ~250-line reformat that buried the one entry that changed —
|
|
# defeating the entire point of committing the baseline. The generator owns the format; a
|
|
# no-op refresh is now a zero-line diff.
|
|
packages/civitai-db-schema/src/schema-drift/drift-baseline.json
|
|
|
|
# Captured by `drift --dump-catalog`. Same reasoning as drift-baseline.json above, and the
|
|
# same failure it prevents: prettier collapses short arrays in a way JSON.stringify will not
|
|
# reproduce, so with prettier owning the format a re-dump of this file was a 21,522-line diff
|
|
# — while the gate's own STALE message instructs the operator to run exactly that command.
|
|
packages/civitai-db-schema/src/schema-drift/__tests__/fixtures/catalog-production-2026-08-03.json
|
|
|
|
# Prisma generate owns the format of these. Prettier re-wraps the long
|
|
# `export type X = (typeof X)[keyof typeof X];` lines it emits, which makes every
|
|
# regeneration a ~700-line diff AND leaves `db:check-generated` permanently red —
|
|
# it regenerates and diffs against the commit, so a formatted commit can never match.
|
|
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/
|