mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Zachary Lowden
e76db50b34
* fix(blocks): stop enumerating capabilities in the ai:write:budgeted consent copy
Operator-confirmed 2026-09-16: an inline customComfy graph CAN generate video.
That makes the sentence #4879 shipped -- "...generating images and running
language models" -- UNDER-name a reachable capability, which is the worse
direction for consent: the user agrees to "images" while the app spends their
Buzz on video.
The copy is now generic:
"Run AI generation services that spend the viewer's Buzz, with a per-call cap"
WHY GENERIC RATHER THAN A BETTER LIST. Enumerating was tried three times and was
wrong three times, each caught by a later audit round:
- "training models" -- over-promised; no implemented billing mode can carry
a variable-cost step, so training cannot be registered
- "and video" -- cut as unreachable on enum reasoning that covered every
arm EXCEPT the one bounded by no enum
- that cut itself -- wrong: customComfy mode:'inline' forwards an arbitrary
graph and the read path applies no media-type check
Each failure costs a re-consent of every live grant, because consent is stored
per (user, app) and no lookup reads a version. A generic term cannot be
falsified by a capability arriving or turning out to be reachable, which is the
property that makes ONE re-consent sufficient instead of a fourth. The guard is
re-pointed to match: it now pins the SHAPE (no modality nouns) rather than
asserting a list of things believed unreachable -- which is what it did before,
with `video` on that list, asserting something false about the product.
Watched red: 3 of 4 assertions fail against the sentence currently on main.
Also fixes the three comment defects flagged during #4879's audit ladder and
deliberately left out of it to avoid widening that diff mid-ladder:
- civitai-redis client.ts and block-registry.service.ts both put the
revocation marker TTL at 15 minutes; it is MAX_BLOCK_TOKEN_LIFETIME_SECONDS
= 14400s (4h), the dev-token lifetime it must outlive
- block-revocation.service.ts said the marker is written when "the publisher
is banned"; no such writer exists -- revokeInstance has exactly two
production call sites, and block-scope.middleware.ts marks publisher-ban
"(Phase 2)"
The SQL's "decide this before you run anything" block is replaced by the answer
and its consequence, including the precondition a merge does NOT satisfy:
civitai deploys from `release`, not `main`, so the generic sentence must be
confirmed rendering in the production consent modal before the grants are
re-taken.
194 files / 4929 tests green; typecheck 0 errors; prettier and eslint clean on
every changed file -- the last of those is the gate I missed on #4879 and only
caught at merge time.
* fix(blocks): use "AI work", not "AI generation services", and record decision 6 as superseded
Round 0 of the audit ladder on this PR found two defects in it. Both are fixed
here.
1. The proposed copy reused the one root word the re-consent exists to retire.
This file's own `.sql` header says, and has said since #4879: "The per-call
cap half is still true. 'Generations' is not: the scope reaches hosted LLM
inference (chatCompletion, registered and live), which is not a generation
in any sense a reader of that sentence would have understood." That sentence
is the entire justification for revoking 15 live grants.
"Run AI generation services that spend the viewer's Buzz" is built on the
same root. On Civitai the word is narrower still -- "Generate" and
"Train a LoRA" are two distinct top-level actions -- so the draft read
NARROWER than the sentence already on release, on both capabilities the
widening actually added. It satisfied "do not enumerate" and still
re-committed the defect it was written to fix.
The copy is now "Run AI work that spends the viewer's Buzz, with a per-call
cap" -- #4879's sentence with only the enumerating clause deleted.
A new guard pins this, because the draft was caught by a human reading it
and nothing else: `does not rebuild the sentence on the word the scope
outgrew` matches /generat/ as a prefix, so generation/generations/
generating/generative are all refused. Watched RED against the rejected
draft, failing with its own assertion message and not to a neighbouring
guard: 3 red / 2 green, the two greens being `per-call cap` and `does not
enumerate capabilities`, both correct since the draft enumerates nothing.
The draft is also added to SUPERSEDED, labelled as never-shipped.
2. The copy silently reversed an operator-taken decision.
Decision 6 of appblocks-no-allowlist-decision-2026-09-15.md §5a required
this sentence to say "an app may train a model on the viewer's Buzz" in
words. Neither the PR body nor any comment recorded that it was being
dropped.
Operator, 2026-09-16: decision 6 is SUPERSEDED, because training is not
reachable -- isBillingModeImplemented accepts 'prepaidFixed' only -- so
naming it would BANK permission for a widening that has not shipped, which
is the silent scope escalation this table exists to prevent. Recorded in
the constant, in the test's guard comment, and in the .sql, each with the
reason and the re-take obligation for when #599 lands.
Also fixes two things in the .sql that the round found actively wrong against
live production:
- The ORDERING block's recognition procedure told the operator to check that
"the new sentence names language models and training explicitly". After the
release cut at 21:49Z, production DOES name language models -- so that test
now PASSES against a sentence the arc has already retracted, which is
verbatim the failure the same file calls "THE ONE WAY TO GET THIS WRONG".
Replaced with a character-for-character match plus the ?ref=release read.
- Two blocks gave opposite standing instructions: one said the sentence names
only what is reachable and grants are re-taken whenever that changes, the
other said a generic term makes ONE re-consent sufficient. Reconciled: the
generic wording removes the MODALITY treadmill; a change of KIND still costs
a new sentence and fresh grants, and training is the live example.
Trimmed ~35 lines of narrative from the constant's comment block, which
duplicated in a seventh place an arc already recorded in SUPERSEDED next to
the test that goes red.
Verification: consent-copy suite 5/5 green; mutation control above; prettier
and eslint clean on both changed TS files, each with a positive control
watched to fire (prettier flags a misformatted probe and prints an explicit
"No files matching the pattern" on a non-match; eslint errors on a planted
no-var) -- this arc has run six rounds without ever running either.
* fix(blocks): stop the comment fixes from re-committing the rot they fix
Round 1 findings 3 and 4, both in the comment-fix half of this PR.
- block-revocation.service.ts: the new docblock says no publisher-ban writer
exists and "do not reason about a ban path from here", and fourteen lines
later the catch comment still said "an uninstall/toggle/ban write path".
A reader landing on revokeInstance saw a ban writer named inside the
function whose docblock had just denied one. Dropped the word.
- client.ts and block-registry.service.ts: both new comments restated the TTL
as a literal "14400s / 4h". block-token-lifetimes.ts says in its own header
that restating it as a number is what let the revocation TTL sit at 15min
for the whole time dev tokens lived 4h -- so the fix for a wrong hardcoded
figure introduced two fresh ones, in the same shape, guarded by nothing:
the live block-revocation-ttl.test.ts asserts ttl >= lifetime derived from
real signed tokens, so raising the dev lifetime keeps every test green and
silently makes both comments wrong in the same direction as the 15-minute
claim. Both already named MAX_BLOCK_TOKEN_LIFETIME_SECONDS; the parenthesised
number is now gone and each says why it is not restated.
Prettier and eslint clean on all three files.
* fix(blocks): drop depth-dependent line cites this PR itself invalidated
Round 2 findings F1 and F4.
F1 is the ladder's own signature failure: the previous round's comment growth
moved the target of a line-number cite onto the semantically OPPOSITE call.
`block-registry.service.ts:2393` held `revokeInstance` inside
`toggleEnabled(false)` on main and at the merge-base; after round 1 (+1) and
round 2 (+1) of comment edits it holds
`await BlockRevocation.clearInstance(...)` -- the revoke CLEAR.
Two files cited it: the one-off SQL's operator-facing runbook, and
`scope-grant.service.ts` (untouched by this PR, so nothing in the diff flagged
it, and neither file is touched on main since the merge-base, so the drift
survives the merge). An operator working the SQL header's "no endpoint exists
whose purpose is revocation" bullet would open :2393, land on the clear, and
conclude that toggling an install off REMOVES markers -- inverting the
containment reasoning that bullet list exists to support.
Fix is to delete the `:NNNN` suffixes rather than re-derive them. The function
names are unambiguous and depth-independent; a line number in a comment is a
cross-reference that goes stale silently and that no test pins.
F4: the client.ts comment told the reader to "read the constant" and named a
bare `block-token-lifetimes.ts`. That file is
`src/server/services/block-token-lifetimes.ts` in the Next app -- a different
workspace package with no import path from `packages/civitai-redis` -- so the
pointer was unfollowable from where it was written. Repo-relative path now,
plus the reason it cannot simply be imported.
prettier + eslint clean on both changed files; eslint's zero earned against a
planted no-var control on the same path and config (round 2 flagged that the
previous round's eslint zero was unproven).
* fix(blocks): sweep the publisher-ban claim, and bound the SQL's ~15min window
Round 3 findings. Two of the three are addressed here; the third was WRONG and
is refuted in the file rather than applied.
## The publisher-ban sweep (round 3's 🟡-3, round 2's F2)
This PR corrected "or the publisher is banned" LOUDLY in
`block-revocation.service.ts` -- "NO SUCH WRITER EXISTS ... do not reason about
a ban path from here" -- and nowhere else, leaving five other sites still
describing a ban as a live revocation-marker writer. That made the tree
self-contradictory in the direction that matters: a moderator reading
`apps.router.ts` would believe banning a publisher kills that publisher's live
block tokens within seconds. It does not; they run to natural `exp`.
Swept all five: `apps.router.ts`, `apps-shared.router.ts`,
`block-bridge-auth.service.ts`, and the two guard tests' header comments. Each
now carries the retraction rather than a bare deletion, so the next reader sees
that the claim was checked and found false. Verified by enumeration: the only
four remaining occurrences of the phrase in `src/` are all retractions.
Round 2 routed this to "the follow-up PR that will gate publishGenerationOutputs
on posts:write:self". Round 3 was right to reject that: no such PR or issue
exists, and adding a scope gate to one endpoint has no reason to touch five
comments about who writes a revocation marker. That was an object with no
closing condition and no owner. Fixing it here instead.
## The ~15 MINUTES window (round 3's 🟡-2) -- REFUTED, and the refutation is
## now in the file
Round 3 read item 1's "900s (300s settings-scoped, 4h dev)" together with its
"up to ~15 MINUTES" conclusion and called the window 16x understated. Round 2
had examined the same thing and cleared it. Round 2 was right:
- dev-token SCOPES do not come from `app_user_scope_grants` at all.
`dev-scoped-mint.service.ts` derives them from the manifest, clamped to
DEV_TOKEN_SCOPE_ALLOWLIST and the app's OAuth ceiling -- so revoking a
consent row cannot narrow a dev token;
- dev-token SPEND never consults consent.
`if (claims.dev === true) return { ...platform, consent: null }` returns
before `getConsentBuzzBudget` is reached.
So a dev token is unaffected by this script in both directions, and waiting 4h
would not close anything. Applying round 3's fix would have made the runbook
wrong in the opposite direction -- telling an operator to wait 16x longer than
the real consent-token window.
What WAS missing is that item 1 never said which population the bound covers.
It does now, with the mechanism and an explicit "do not fix this number upward",
because two audit rounds disagreed about it and the next one should not have to
re-derive it.
Verification: 41 tests across the consent-copy, revocation-TTL, ban-session and
bridge-token guard suites pass. prettier/eslint on the changed files add no new
findings -- `apps.router.ts` is unformatted on HEAD already (confirmed against a
clean checkout) and carries 7 pre-existing no-empty-function errors before and
after this edit, so the edit was kept minimal rather than running prettier
--write, which would have reformatted 787 lines of unrelated code.
* fix(blocks): sweep the ban claim by MEANING, not spelling, and fix the authority it cites
Round 4 findings. The previous round's sweep was the finding this time.
## The sweep matched a SPELLING, and the claim has another word order
`no-unguarded-block-bridge-token.test.ts:940` still read "a revoked install, a
suspended app and a banned publisher all still drive them until the token
expires". My enumeration matched `publisher ban` and `publisher-banned`;
"a banned publisher" is neither, so it survived a sweep that reported itself
complete -- in the same file whose header comment 930 lines above it WAS swept.
It is also the worst surviving instance, because it is a live assertion
message rather than a comment: it prints when the guard test goes red, so a
developer adding a bridge proc reads it at exactly the moment they are deciding
what to do, and it tells them routing through authorizeBlockBridgeToken
contains a banned publisher. It does not -- the guard's three steps are token
validity, the revocation marker and app_blocks.status, and toggleBan writes
none of the three.
Re-enumerated by MEANING this time (any word order, case-insensitive, over src/
and packages/, with a positive control confirming 141 files match "publisher"
so the pipeline was live). Every remaining co-occurrence is a retraction or the
unrelated PublisherSubscriptionBanner component.
## The authority four retractions cite said the opposite
Two retractions offered `block-scope.middleware.ts`'s "(Phase 2)" marking as
their ONLY evidence, and that line read "Uninstall, toggleEnabled(false), and
(Phase 2) publisher-ban all write a marker" -- main verb asserting all three DO.
"(Phase 2)" cannot carry "this one does not", and it does not mean unshipped in
this tree: dev-scoped-mint.service.ts and blocks.router.ts both label live,
tested surfaces "(Phase 2)". A reader following the pointer landed on a
contradiction of the thing it was cited for.
That line now states the enumeration itself -- two production call sites, named
-- so the citations resolve to evidence rather than to a label.
## DEV_BUZZ_BUDGET_CAP was paired with the wrong reason
The SQL said item 2's cap-lift cannot compound onto a dev token because it
"carries its own DEV_BUZZ_BUDGET_CAP". That cap is 250 PER CALL, not a daily
bound, and a dev token still reserves against the full platform per-day ceiling.
The actual reason is the claims.dev short-circuit named in the clause before it.
Reworded so an operator cannot read it as "dev tokens are capped at 250/day",
and the dev-token holder population widened from "the mod" to authors too, who
hold them via the cookie dev-tunnel branches.
Verification: 34 tests across the bridge-token-guard, revocation-TTL and
consent-copy suites; prettier and eslint clean on both changed files.
Also noting, since it cannot be amended into the commit it belongs to: the
"291 rules" figure in the round-3 correction comment does not reproduce --
measured 285-287 depending on the src/ path. The load-bearing half (2 rules and
no no-var under the packages/ root:true boundary config) reproduces exactly.
* fix(blocks): finish the ban-claim citation chain, and stop asserting a false universal
Round 5 findings, all four.
A. Four files cited "block-scope.middleware.ts marks publisher-ban (Phase 2)"
as their evidence. Round 4 rewrote that line -- and the replacement
explicitly says "(Phase 2)" could NOT carry that meaning and labels shipped
things elsewhere in this tree. So every pointer cited an authority that had
just repudiated the inference form the citation used. All four now carry the
two-call-site enumeration instead of the label.
B. A false universal this ladder introduced last round, in the block those four
files are pointed at: "toggleBan (user.service.ts) calls only
invalidateSession". It is a ~250-line function that also unpublishes every
model the user owns, cancels their subscription, blocks their media, deletes
their user links and flags their comments. A maintainer asking "what does
banning do?" would have read the authoritative-looking enumeration and
concluded a ban only logs you out. The correct narrow form was five lines
away in the test message the same commit wrote: it writes none of the THREE
THINGS THIS GUARD CHECKS. Now says that, and says explicitly not to read it
as "a ban only logs you out".
C. `appblocks-no-allowlist-decision-2026-09-15.md` was cited three times,
including from the payload, as though it were a path in this repo. It is not
in this repo at all -- it lives in the private civitai/talos-infra clone. The
one operator who most needs to check the superseded decision could not open
it and had no way to know why. All three sites now name the repo.
D. Round 4's fix put four lines of dated retraction history into a LIVE
assertion message -- the surface round 4 had just identified as the worst
place for exactly this, because it prints when CI goes red. Moved to the file
header; the message is back to the instruction a developer needs.
Verification: 34 tests across bridge-token-guard, revocation-TTL and
consent-copy. prettier clean on all 7 touched files. eslint reports 10 errors,
all pre-existing `no-empty-function` in apps-shared.router.ts (same count as
before this edit; line numbers shifted +1 by an added comment line) -- read
without a pipe, since `eslint | grep; echo rc=$?` reports grep's status.
This is the last round. Rationale in the PR comment: rounds 3-5 have audited
prose the ladder itself wrote, the executable payload has been unchanged since
57b9f8638e, and this round's fixes twice broke grammar in sentences written to
fix earlier grammar. Remaining known-imperfect items are recorded there rather
than fixed, so the next reader sees them as open rather than absent.