mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Zachary Lowden
720ed3e087
* fix(app-blocks): make /api/v1/blocks/me and blocks.getMyViewer agree on authorization
Two front doors to one capability disagreed about who may read viewer identity,
and the disagreement was masked by the Flipt audience rather than absent.
GET /api/v1/blocks/me carried a hardcoded isModerator -> 403 ("Phase 2: App
Blocks is moderator-only until GA"), no App-Blocks flag gate and no rate
limiter. Its tRPC twin blocks.getMyViewer had the flag gate and the rate
limiter, no moderator literal, and a docblock claiming it mirrored me.ts
EXACTLY. The live app-blocks-enabled audience is mostly moderators, for whom the
literal refused nobody the flag would have admitted -- but it also holds
hand-allowlisted non-moderators, and for every one of them the REST door 403'd
while the bridge returned 200. Widening the audience makes that the general
case.
The decision (operator, 2026-09-18) is that Flipt is the gate; a code-level
availability:['mod'] is documented as a Flipt-DOWN fallback only. So:
- Drop the moderator literal from me.ts, and the isModerator column from its
select.
- Move assertAppBlocksEnabledForTokenUser out of blocks.router.ts into
src/server/services/blocks/block-token-access.service.ts so both doors run ONE
implementation. A Next API route cannot import the tRPC router, and a second
copy of the predicate is how the two came to disagree.
- Give me.ts that gate plus checkBlockCatalogRateLimit, same bucket, same
position (before the primary read) as getMyViewer.
- Replace the false "mirrors EXACTLY" docblock with explicit SHARED and
NOT-SHARED lists, where SHARED is exactly what the parity test exercises and
NOT-SHARED names the three places the doors genuinely differ -- including two
pre-existing bridge-side divergences this change records rather than fixes.
- Repoint scripts/compiled-branch-watchlist.mjs's module: the watchlisted
fail-closed branch block-token-subject-refusal moved with the function.
me.ts renders both kill-switch refusals with its own literal and does NOT echo
the gate's message: rest-error-envelope-ledger.test.ts blocks a REST route from
serialising a caught error's .message, and the unhydratable-subject message is a
compiled-branch anchor that must stay unique app-wide. Detection is duck-typed
on .code rather than instanceof, matching the sibling routes, because
instanceof fails across a duplicated @trpc/server instance in an API bundle.
Tests: blocks.router.me-parity.test.ts drives BOTH doors with one subject and
compares a normalised verdict. Measured in a CLEAN checkout of the base commit
with the HEAD test files dropped in -- at 0340f692bf, then re-measured at
d7038c5aa8 after the base moved, 7 failed / 3 passed both times: 7 of 10 FAIL, in
BOTH directions -- REST too strict for a non-moderator in the audience (allowed,
banned, muted), REST too permissive for a moderator outside it and for an
over-limit instance, one door not calling the gate at all, and a non-tRPC error
not being rethrown. The other 3 are labelled at their own assertions as invariant
or mutation guards and are NOT counted as regression coverage -- including one
that is green at base only because both doors answer 403 there by coincidence.
Mutation-tested rather than assumed. Adversarial review found three mutants that
SURVIVED an earlier revision of the suite and they are now killed by cases added
for them: `toHaveBeenCalledWith` is satisfied by EITHER door when both doors call
one mock, so keying a limiter on `jti` or calling the gate with a wrong subject
went unseen (both assertions now compare `mock.calls`); hardcoding the refusal
status instead of deriving it went unseen because every refusal the gate can
currently produce is UNAUTHORIZED (case H); and widening the duck-typed catch to
`if (true)` went unseen, which turned a plain Error into a 500 wearing a policy
refusal's body (case I).
Does NOT widen the Flipt segment; that is a separate change this one unblocks.
* test(app-blocks): pin the gate/limiter ORDER, label the third invariant guard, ledger the exported kill-switch
Audit round 1 on #4950 cleared the payload and found three gaps in the GUARDS
around it. All three are test/doc-side; no production behaviour changes here.
F1 (the one that matters) — A SURVIVING MUTANT, on exactly the defect this PR
exists to fix. Three docblocks claim the parity test pins the kill-switch and the
rate limiter in the same ORDER on both doors ("same position", "same placement",
asserted to be "exactly what the parity test EXERCISES"). It did not. Measured:
hoisting the limiter block above the kill-switch try/catch in me.ts left the
parity file 10/10 and 16 sibling suites 417/417 green. 13 of 14 audit mutants
died; this one lived.
The reason is that every existing case arms at most ONE of the two refusals, and
with one armed the order is unobservable -- whichever gate is armed answers,
wherever it sits. Position relative to the PRIMARY READ was already pinned (B and
D assert the db was never touched, which killed a gate-after-db mutant); position
relative to EACH OTHER was not.
Fixed by adding the case, not by narrowing the words: case J arms BOTH (flag
false AND limiter denied) and asserts the two doors return the same verdict --
401 "Apps are not enabled", the kill-switch, because it runs first -- plus the
positive half, that NEITHER door reaches the limiter at all. Re-running the exact
mutation now fails exactly case J, for its own reason (verdict inequality):
before 26/26 green, after 1 failed / 25 passed.
A docblock claiming coverage the test lacks is this PR's whole thesis. It would
have been one more instance of it, in the file arguing against it.
F2 — the parity header said its three green-at-base cases are "each labelled at
its own assertion". E was labelled in its title and H in its body; G was not, so
a reader landing on G from a failure got no in-place signal it is an invariant
guard rather than regression coverage. Now labelled in both.
F3 — `assertAppBlocksEnabledForTokenUser` became exported by this PR, and its
contract (the id MUST be the self-bound token subject) stopped being checkable by
reading one file. Nothing enumerated its callers: the bridge reachability guard
covers a different function. Adds a call-site ledger that pins the consumer SET
and per-consumer call counts, failing when the set grows, shrinks, or a new call
site appears inside an existing consumer.
Resolution is by IMPORT, never by name: apps.router.ts declares its own
same-named `(userId, op)` variant that is a deliberate documented divergence, and
a name-matching ledger would have counted it and invited someone to "reconcile"
two functions that are separate on purpose. That module is asserted as an
explicit negative control, alongside a positive control so a matching set cannot
be a wired-to-nothing zero.
The ledger states its own limit plainly: it does NOT verify self-binding. Proving
an argument descends from parseSubjectUserId textually is not something a regex
does honestly, and a structural check type-checks past a wrong argument anyway.
What it buys is that a new caller cannot land silently -- it lands in a diff next
to the contract. Watched to fail in all three directions before being trusted.