mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
fix(app-blocks): a no-user flag eval returns the flag's BASE value, not a deny (#4807)
* fix(app-blocks): a no-user flag eval returns the flag's BASE value, not a deny
Several docblocks in app-blocks-flag.ts derived a security property from an
inference that does not hold:
no user -> a global eval that can never match a segment -> fail-closed
The premise is true. A no-user call reaches Flipt as entityId 'global' with an
empty context, and every identity/tier/cohort segment we have is a
STRING_COMPARISON_TYPE constraint that reads the context, so none can match. The
conclusion does not follow from it: when no rollout matches, Flipt answers with
the flag's own base 'enabled' value. The denial came from the BASE being false,
not from the segment miss — so a base-true widening of app-blocks-author or
app-blocks-enabled turns every no-user branch from a deny into a pass.
MEASURED against the real @flipt-io/flipt-client-js wasm engine over a real
evaluation snapshot (new test app-blocks-flag.base-enabled-flip.test.ts):
base enabled:true + a non-matching SEGMENT_ROLLOUT, no entityId/context -> true
base enabled:false + the same rollout, no entityId/context -> false
unknown flag key, no entityId/context -> false
Both flags are base-false with segment rollouts today (civitai/flipt-state,
civitai-app/default/features.yaml), so nothing is exposed now. This is the latent
gate closed before any base-true flip, not a live defect.
Code:
- isAppBlocksAuthorEnabled: an undefined user now returns false structurally
instead of falling through to a global eval. Enumerated: all 10 call sites pass
{ user }; none wants a global eval of this key.
- blocks.router assertAppBlocksEnabledForTokenUser / assertViewerIsAppDeveloper:
refuse an unhydratable token subject before consulting the flag, with a distinct
message. Same shape apps.router.ts already uses.
- apps-shared.router resolveSharedContext: separate a VANISHED subject (refuse)
from an ANON token (keep the global eval — that widening is intended). The
read ops list/get had no second belt behind the flag.
- isAppBlocksEnabled's no-user branch is deliberately KEPT: it has a real caller
(pages/api/v1/developer/block-manifests.ts) that wants the base value.
Comments: every fail-closed / fail-safe claim in app-blocks-flag.ts swept and
given an accurate statement of what makes it closed; a new GLOBAL-EVAL SEMANTICS
block records the mechanism and the measurement once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(app-blocks): make the compiler the guard — require a subject on the author capability
Round-1 review: the structural guard I said needed type-level nullability analysis
is analysis this repo already runs on every PR (tekton / typecheck, App unit tests
+ typecheck). Measured, then implemented.
isAppBlocksAuthorEnabled's parameter is now REQUIRED and non-nullable:
(opts?: { user?: SessionUser }) -> (opts: { user: SessionUser })
Making it required errored at exactly 2 of the 10 call sites — both bare
middleware(...) whose ctx.user is not narrowed by the protectedProcedure they are
attached to (app-listings.router.ts, app-collaborators.router.ts). Each now refuses
explicitly instead of handing a possibly-undefined subject to an authz gate. The
other 8 already held a non-null subject. The runtime `if (!user) return false`
branch this makes dead is DELETED rather than kept as defence in depth, and the two
runtime tests that pinned it are deleted with it: they could only be re-added behind
an `as never` cast, i.e. testing a path the type system forbids while reading as
coverage. What replaces them is the typecheck gate, plus one test pinning the
residual the docblock now states — a cast-defeated call THROWS, never returns true.
isAppBlocksEnabled keeps its optional overload. That asymmetry is now justified on
SEMANTICS (a kill-switch answers "is the feature on at all", which a subject-less
machine path may legitimately ask and which the base value is; a capability answers
"may THIS subject", unanswerable without one) rather than on its sole no-arg caller
existing — block-manifests.ts is dormant, so that justification is the half that can
vanish.
Also:
- Delete fixtures/flipt-base-enabled-flip.snapshot.json (152 lines). Derived at
runtime from the sibling fixture by re-key + enabled flip. A checked-in twin cannot
track the original — re-capturing the source means re-anonymising it, so the copy
silently keeps the old segment shape while claiming production fidelity, and it had
already lagged a segment.
- Consolidate ~60 lines of duplicated harness into fixtures/flipt-fixture-server.ts,
shared with the pre-existing real-flipt-client integration suite. Only the snapshot
differs between them; the instrument is the same.
- Stop enumerating live flag state in the GLOBAL-EVAL SEMANTICS header — that is the
same rot class the paragraph warns about. Keep the imperative, point at flipt-state.
- apps-shared: say READ_OPS rather than naming two of its four members.
- Replace a rejects.not.toMatchObject(...) positive control, which passes on any
other rejection, with the specific NOT_FOUND / 'Block install not found' outcome.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(app-blocks): watchlist the two new refusals — the type guard cannot see a bundler
Round-1 review, 2 x yellow. Both taken.
WATCHLIST (the important one). My stated defence for deleting the helper's runtime
branch — "deleting it is a type error rather than a silent re-opening" — is a claim
about SOURCE. Release 5.1.18 is the case where the source is correct and the emitted
artefact is not: the bundler dropped two of three returns from a function in
app-blocks-flag.ts and served the whole App-store catalog to anonymous callers
(civitai#3983). That is why scripts/compiled-branch-watchlist.mjs exists, enforced at
Dockerfile:103, and two of the three refusals this PR adds are pure runtime branches
whose loss silently restores the exposure it closes. Both are now listed:
shared-storage-subject-refusal apps-shared.router.ts — lost, every READ_OPS op
serves shared rows to a vanished subject
block-token-subject-refusal blocks.router.ts — lost, the kill-switch is
evaluated with no subject on 16 runtime procs
assertViewerIsAppDeveloper's guard is deliberately NOT listed, and that is measured:
isAppBlocksAuthorEnabled takes a non-nullable subject and dereferences it at once, so
losing that guard throws rather than passes.
VERIFIED AGAINST A REAL BUILD, not just added. Full `next build` (green), then the
gate exactly as the Dockerfile runs it:
positive 26,600 maps scanned; both new entries report OK with their control
anchors mapped (an unmapped control is exit 2, not a pass)
negative required anchors repointed at unmapped comment lines in the same
functions -> exit 1 naming both entry ids, controls still mapping
deletion the required line removed in a throwaway source copy, one entry at a
time -> exit 2 naming that entry and its anchor
The two refusal messages were identical strings under different codes, so neither was
anchorable and "separable in a log" was not true. The author-gate message is now
'app-authoring subject could not be resolved'.
ROT (the second yellow). The isAppBlocksAgenticReviewEnabled docblock I wrote for this
rot class had the rot: it called the absent-flag half "load-bearing while this flag
does not exist in Flipt", but app-blocks-agentic-review has existed since 2026-07-21
(live: base false, `moderators` segment). No live exposure — all three call sites
check isModerator first — but it is a false claim in the one paragraph my own sweep
rewrote. Re-derived every app-blocks flag's live state rather than fixing only the
reported line, and found a second instance I had authored: the dev-tunnel docblock
offered "absent" as a live unconditional closure for a flag that also exists. Both
corrected; the only flag in this file genuinely absent from Flipt is
app-blocks-backpay-enabled.
Nits: assertAppBlocksEnabledForTokenUser has 16 call sites, not 17 (17 is the
parseSubjectUserId count; the substantive claim was right). deriveSnapshotFromFlagShape
now rejects an EMPTY rollouts array, which its error text already claimed to reject.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(app-blocks): resolve the two docblocks that answer the reader twice, oppositely
Follow-on from the same re-check that found the agentic-review rot. Eleven docblocks
in this file carry an inherited "the flag does NOT exist in Flipt at merge time"
sentence; the new header frames all of them as as-merged history, which is the
deliberate treatment (substituting a fresher enumeration would be the same rot one
generation on).
But in exactly TWO docblocks that inherited sentence sits in the SAME block as a
live-state sentence I wrote, so the reader gets opposite answers eight lines apart:
isAppListingsEnabled "does NOT exist" vs "Both are base-`false` today"
APP_BLOCKS_SHARED_STORAGE_FLAG "does NOT exist" vs "Closed today because the base
is `false`"
Both inherited sentences moved to past tense and marked as as-merged notes. Nothing
else changed: the other eleven are untouched, because they carry no competing claim.
Criterion for the split, so it can be re-applied: fix where a paragraph contradicts
itself; leave where the header already governs.
Re-verified after the edit rather than carrying the previous measurement over — these
are comments, but they shift line numbers in a WATCHLISTED module, and source/artefact
correspondence is exactly what this PR stopped assuming. Fresh `next build` (green),
then the compiled-branch gate: positive OK on all three entries (26,600 maps; controls
mapped), negative control still exit 1 naming both new entries.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(app-blocks): say at each watchlisted refusal that it is watchlisted
The compiled-branch gate names the entry, module, line and reason when it fires — but
only at Docker build time, after someone has already deleted the branch and pushed. The
two refusals this PR puts on the watchlist carried no signal in the file itself, so a
reader deciding whether the guard is load-bearing had nothing to go on. Two comments,
one at each site, naming the entry id and what a bundler can do to a pure runtime
branch.
Deliberately scoped: moving or rewording the throw is fine (the gate resolves its
anchor from source at run time), deleting it is not.
Re-verified after the edit, for the same reason as the previous commit: comments shift
line numbers in watchlisted modules. Fresh `next build` (green), gate positive OK on
all three entries, negative control still exit 1 naming both new entries. That is the
third independent repetition of both controls.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(app-blocks): re-anchor the watchlist on format-robust text, and stop telling people to reword it
Round 2. Five defects, all introduced by this PR's own later commits.
F1 (the important one). The comment I added in 1f3fd96 said "Moving or rewording the
throw below is fine". Moving is fine; rewording is NOT, and neither is reformatting —
the anchor is an exact substring, a miss is die(2), and exit 2 is deliberately not
downgraded by --warn-only, so the first red is the production image build. Worse, both
anchors baked in the FORMATTING (a trailing `});` and a trailing `,`), and the
apps-shared line was 95 chars at printWidth 100 — so adding a `cause` or a metrics
counter, which apps.router.ts already has, reflows it and reds the build.
Fixed at the anchor, not just in the prose:
shared-storage required -> `if (userId != null && !subjectUser) {` (the CONDITION)
control -> `isAppBlocksSharedStorageEnabled(` (stops at the paren)
block-token required -> `'runtime block token subject could not be resolved'`
control -> `await isAppBlocksEnabled({ user })`
Measured, resolution phase isolated (it runs before any map is read):
OLD anchors + old source resolution OK (control)
OLD anchors + the apps-shared reflow RESOLUTION FAILED
NEW anchors + the same apps-shared reflow resolution OK
NEW anchors + apps-shared message reword resolution OK (anchor is the condition)
NEW anchors + blocks.router property-add resolution OK
NEW anchors + blocks.router message reword RESOLUTION FAILED <- inherent, documented
NEW anchors + unmodified copy resolution OK (control)
The last row cannot be fixed by choosing better text: `if (!user) {` is not unique in
blocks.router.ts (the author gate uses the same condition), so that entry must anchor a
literal. It is sound only because the literal is now unique APP-WIDE — nothing can
intern it from elsewhere — and that constraint is written into the entry, the site
comment and a new anchor-authoring rule in the watchlist header.
F5, which is the same edit. The kill-switch refusal was byte-identical in code AND
message to apps.router.ts:148, so an operator could not tell which gate refused.
Renamed to 'runtime block token subject could not be resolved' — the message is new in
this PR, so no existing consumer sees the change — which clears the collision and
supplies the unique literal F1 needs.
F2. 220fdd8e0 created a new "answers the reader twice, oppositely" by its own criterion:
the dev-tunnel docblock lost the absent-flag case three lines above an untouched
sentence that relies on it. That sentence is a live-state claim, so the file header does
not govern it. Made state-independent, matching the treatment its sibling edit in the
same commit already used.
F3. The nit fix swapped one wrong count for another. There are 16 parseSubjectUserId
sites, not 17: the site at :5674 guards BOTH gates, so the two sets overlap and must not
be added. (19 raw occurrences = 16 calls + the import + two comments.)
F4. The fixture guard checked only that some rollout exists while its message and the
consuming test's title both claim a SEGMENT rollout — so a re-capture with a threshold
rollout would pass, and a 100% threshold would sail past the base-false backstop too.
Now checks SEGMENT_ROLLOUT_TYPE, and a new test drives all three refusal arms plus a
positive control, so the guard is reachable rather than merely present.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(app-blocks): round 3 — stop writing the count down at all, and widen the fixture guard to its own docstring
Round 3 returned no 🔴, two 🟡 and two 🟢. This fixes both 🟡 and the 🟢 that is a
false claim in a doc governing how anchors are authored. Each was re-measured here
before being accepted; the auditor was right on all three.
🟡-1. The previous commit's "fix by FORM" was self-falsifying. It replaced a wrong
total with `grep -c 'await assertAppBlocksEnabledForTokenUser'` -> 16, and that line
CONTAINS the pattern, so it matches itself:
31d768fba2 (base) 16
5ad903c5c1 16
origin/main 16
7f029a6947 (merge) 17 <- the comment counted itself
Run without a path constraint it is 18, because apps.router.ts:139 declares a
DIFFERENT function of the same name. So a maintainer reading "16 such parse sites"
and running the very next line would get 17, conclude the doc was stale by one, and
reinstate the off-by-one that round 1 introduced and round 2 wrote the paragraph to
kill — the fourth consecutive wrong count in one parenthetical.
No number and no command now. The paragraph says why both are unsafe here and tells
the reader to enumerate. It also records that it has been wrong four rounds running,
each time by writing a figure down, so the next editor knows the cost before reaching
for a fifth.
Note for the record: the merge commit's own justification for that rewrite was wrong
in two specifics — it named `assertAppBlocksEnabledForTokenUser` where the count is
about `parseSubjectUserId`, and said the "import" did not exist when it does (line
15). Not force-pushed; the correction lives here beside it.
🟡-2. `flipt-fixture-server.ts` checked `.some(r => r.type === 'SEGMENT_ROLLOUT_TYPE')`
while its own comment names the hazard as a THRESHOLD rollout. `.some()` does not
cover a threshold rollout captured ALONGSIDE the segment one: the segment rollout is
still present, the guard passes, and `base-false-control` then evaluates true for
every subject — the exact attribution failure the fixture exists to prevent, restored
by a routine snapshot re-capture with a green suite. A guard whose description was
wider than its implementation.
Now requires a segment rollout AND no threshold rollout, with DISTINCT text per arm so
neither can die to the other's error. The test grew the arm that was uncovered, and
each arm now carries its own expected message rather than sharing one regex.
🔴 The first cut of this fix was itself wrong and an existing test caught it: keying
the message on `hasThreshold` alone made a THRESHOLD-ONLY template announce a rollout
"alongside its segment rollout" that was not there. `hasSegment &&` is load-bearing;
the comment says so.
Mutation-tested, each dying for its OWN reason on a DIFFERENT arm:
M1 drop the threshold arm (pre-fix behaviour) -> red: "expected [Function] to throw an error"
M2 key the message on hasThreshold alone -> red: message mismatch on the threshold-only arm
M1 is red at the pre-change code, so the new arm is regression coverage rather than an
invariant guard.
🟢-3. `compiled-branch-watchlist.mjs` asserted "comment churn above it cannot break
it", full stop. False: `resolveAnchor` scans every line including comments and >1
match is a hard error, so a comment merely CONTAINING the anchor text breaks
resolution exactly as a reword does — and rule 1's "prefer the shortest fragment"
makes a collision likelier, not less. This sentence had already misled a reader into
treating a docblock edit in a watchlisted module as free. Corrected, and it now says
the first red is this gate's own unit suite rather than the image build, which is the
only reason it is not deploy-blocking.
NOT fixed, deliberately: 🟢-4, that F5's new message is a strict SUPERSET of the
sibling it was renamed to be separable from, so a Loki grep for the shorter string
still returns both. The fix is another reword of a WATCHLISTED anchor plus its two
consumers, which is not a change to make at the tail of a ladder to close a 🟢.
Recorded as a ranked item instead.
Verified: 5 suites / 163 tests green (assert-compiled-branches, app-blocks-flag,
app-blocks-flag.base-enabled-flip, blocks.router.flag-gate-hydrate,
apps-shared.router); `pnpm typecheck` 0 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F9mHaY8iuJYDNfgYeSeWNj
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Zachary Lowden
@@ -51,6 +51,37 @@
|
||||
* not observe the function at all (exit 2) instead of claiming a violation.
|
||||
* Without one, a module that simply was not emitted reads as N violations.
|
||||
*
|
||||
* 🔴 CHOOSING THE ANCHOR TEXT — the real rot vector, and it is NOT line numbers.
|
||||
* Anchors are resolved to a line at run time, so an anchor survives the code moving,
|
||||
* and comment churn above it cannot SHIFT it. What DOES break it is the anchored TEXT
|
||||
* changing, and a miss is `die(2)` — which `--warn-only` deliberately does not
|
||||
* downgrade, so the first red is the production image build.
|
||||
*
|
||||
* 🔴 An earlier revision of this paragraph said "comment churn above it cannot break
|
||||
* it", full stop. That is FALSE and it misled a reader into treating a docblock edit in
|
||||
* a watchlisted module as free. `resolveAnchor` scans EVERY line, comments included, and
|
||||
* more than one match is also a hard error — so a COMMENT that merely CONTAINS the
|
||||
* anchor text breaks resolution just as a reword does. Rule 1 below makes this likelier,
|
||||
* not less: the shortest fragment is the easiest for prose to collide with. It is caught
|
||||
* by this gate's own unit suite rather than first at the image build
|
||||
* (`scripts/__tests__/assert-compiled-branches.test.ts` asserts every anchor resolves to
|
||||
* exactly one line), which is the only reason it is not a deploy-blocking trap. Two
|
||||
* rules follow:
|
||||
*
|
||||
* 1. Anchor a substring that survives REFORMATTING. Do not include a trailing `,`
|
||||
* or `});`, and do not anchor a whole long line: a line near `printWidth` reflows
|
||||
* the moment anyone adds a property (`cause`, a metrics counter), and prettier
|
||||
* then splits it. Prefer the shortest unique fragment — stopping at an open
|
||||
* paren is a good trick, since argument reflow keeps the callee on its own line.
|
||||
* 2. Prefer the branch's CONDITION over a payload inside it. A string literal can in
|
||||
* principle be interned from elsewhere and keep a mapping while the branch around
|
||||
* it is eliminated; a condition cannot. Where a condition is not unique in the
|
||||
* module, a literal that is unique APP-WIDE is an acceptable substitute — see
|
||||
* `block-token-subject-refusal` — because nothing else can intern it.
|
||||
*
|
||||
* Rewording an anchored message is therefore a watchlist edit too, not just a copy
|
||||
* change. That is the price of the gate, and it is cheap next to the defect it catches.
|
||||
*
|
||||
* Keep this list SMALL and justified — a fail-closed branch whose loss changes who can
|
||||
* see what. Every entry must say what goes wrong when the branch disappears.
|
||||
*/
|
||||
@@ -76,4 +107,38 @@ export const COMPILED_BRANCH_WATCHLIST = [
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: 'shared-storage-subject-refusal',
|
||||
module: 'src/server/routers/apps-shared.router.ts',
|
||||
why: "`resolveSharedContext` refuses a block token whose subject no longer hydrates, BEFORE consulting `app-blocks-shared-storage`. Lost, an unresolvable subject falls through to `{ user: undefined }` — a global eval, which returns the flag's BASE value, not a deny. Under a base-`enabled: true` GA flip every op in READ_OPS then serves shared rows to a token whose subject is gone. The write path is covered downstream by the min-trust gate; the read ops skip that block entirely and have no second belt, so this branch is the only thing in front of them.",
|
||||
control: [
|
||||
{
|
||||
code: 'isAppBlocksSharedStorageEnabled(',
|
||||
why: 'the flag call immediately after the refusal — same function, known to survive. Unmapped means this gate is looking at a build that never emitted `resolveSharedContext`, not at a violation. Deliberately stops at the open paren so reflowing the arguments cannot move it off this line.',
|
||||
},
|
||||
],
|
||||
required: [
|
||||
{
|
||||
code: 'if (userId != null && !subjectUser) {',
|
||||
why: "the refusal's own CONDITION — the branch itself, not a payload inside it. Lost, the next line evaluates the flag with no subject and the answer becomes the flag base. The `userId != null &&` half is load-bearing in the other direction: without it a genuine anon token (`sub:'anon'`) would be refused too, which is the GA widening this gate must NOT block.",
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: 'block-token-subject-refusal',
|
||||
module: 'src/server/routers/blocks.router.ts',
|
||||
why: "`assertAppBlocksEnabledForTokenUser` refuses an unhydratable token subject BEFORE consulting `app-blocks-enabled`. Lost, it falls through to `isAppBlocksEnabled`'s no-user branch — a deliberate global eval kept for the machine registrar — which returns the flag's BASE value. Under a base-`enabled: true` GA flip a token whose subject no longer resolves then passes the kill-switch on 16 block-token runtime procs. NB the sibling `assertViewerIsAppDeveloper` guard is deliberately NOT listed: `isAppBlocksAuthorEnabled` takes a non-nullable subject and dereferences it at once, so losing that one throws rather than passing.",
|
||||
control: [
|
||||
{
|
||||
code: 'await isAppBlocksEnabled({ user })',
|
||||
why: 'the flag call immediately after the refusal — same function, known to survive. Unmapped means the build never emitted this function.',
|
||||
},
|
||||
],
|
||||
required: [
|
||||
{
|
||||
code: "'runtime block token subject could not be resolved'",
|
||||
why: "the refusal's message literal. Lost, the gate evaluates the kill-switch with no subject and a base-true flag answers `true`. NB this anchors a literal INSIDE the branch rather than the branch's condition, because `if (!user) {` is not unique in this module (the author gate above uses the same condition). That is sound here only because the literal is unique ACROSS THE WHOLE APP: a minifier cannot intern it from another site, so a surviving mapping for this line means this site survived. Keep it unique — do not reuse this string elsewhere.",
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
@@ -273,6 +273,38 @@ describe('H3 min-trust gate (write + vote)', () => {
|
||||
expect(out.items).toEqual([]);
|
||||
});
|
||||
|
||||
it('🔴 a VANISHED subject is refused on a READ even with the flag base-`enabled: true`', async () => {
|
||||
// The READ ops have no second belt: `append`/`vote` catch a vanished subject on
|
||||
// the min-trust gate above, `list`/`get` never reach it, so the shared-storage
|
||||
// flag was the only thing standing there — and its no-user branch is a GLOBAL
|
||||
// eval, which returns the flag's BASE value rather than a guaranteed `false`.
|
||||
// `mockIsSharedEnabled` is forced TRUE here to model the GA base flip; before the
|
||||
// fix, `list` resolved and served shared rows to a token whose subject is gone.
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ sub: 'user:999' }));
|
||||
mockGetSessionUser.mockResolvedValue(null);
|
||||
mockIsSharedEnabled.mockResolvedValue(true);
|
||||
await expect(caller().list({ blockToken: 't' })).rejects.toMatchObject({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'token subject could not be resolved',
|
||||
});
|
||||
await expect(caller().get({ blockToken: 't', key: 'k' })).rejects.toMatchObject({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'token subject could not be resolved',
|
||||
});
|
||||
});
|
||||
|
||||
it('POSITIVE CONTROL: an ANON token still READS under the same base-true flag', async () => {
|
||||
// The anon path must NOT be swept up by the refusal above — `sub:'anon'` has no
|
||||
// subject to vanish, and a global eval of a base-enabled flag is precisely the
|
||||
// intended GA widening. Without this, the previous test is indistinguishable from
|
||||
// a change that simply closed shared reads.
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ sub: 'anon' }));
|
||||
mockGetSessionUser.mockResolvedValue(null);
|
||||
mockIsSharedEnabled.mockResolvedValue(true);
|
||||
const out = await caller().list({ blockToken: 't' });
|
||||
expect(out.items).toEqual([]);
|
||||
});
|
||||
|
||||
it('anon NEVER writes (UNAUTHORIZED)', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValueOnce(validClaims({ sub: 'anon' }));
|
||||
await expect(caller().append({ blockToken: 't', value: { title: 'x' } })).rejects.toMatchObject(
|
||||
|
||||
@@ -237,20 +237,94 @@ describe('assertAppBlocksEnabledForTokenUser — Flipt context is hydrated from
|
||||
expect((appBlocksCall as [string, string, Record<string, string>])[2].tier).toBe('gold');
|
||||
});
|
||||
|
||||
it('a vanished subject → undefined user → global eval → flag false → blocked (fail-closed preserved)', async () => {
|
||||
it('a vanished subject is refused BEFORE the flag is consulted (no Flipt call at all)', async () => {
|
||||
mockGetSessionUser.mockResolvedValue(undefined as never);
|
||||
|
||||
const caller = blocksRouter.createCaller(fakeCtx() as never);
|
||||
await expect(
|
||||
caller.pollWorkflow({ blockToken: 'tok', workflowId: 'wf_1' })
|
||||
).rejects.toMatchObject({ code: 'UNAUTHORIZED', message: 'Apps are not enabled' });
|
||||
).rejects.toMatchObject({
|
||||
code: 'UNAUTHORIZED',
|
||||
message: 'runtime block token subject could not be resolved',
|
||||
});
|
||||
|
||||
// Global eval: with no user, isAppBlocksEnabled takes the no-user branch and
|
||||
// calls isFlipt(flag) with NO entityId/context (buildFliptContext is never
|
||||
// run) → the moderators-segmented default stub resolves false.
|
||||
const appBlocksCall = mockIsFlipt.mock.calls.find((c) => c[0] === 'app-blocks-enabled');
|
||||
expect(appBlocksCall).toBeDefined();
|
||||
// No context argument was passed (global eval), so the segment can't match.
|
||||
expect((appBlocksCall as unknown[])[2]).toBeUndefined();
|
||||
// 🔴 THIS ASSERTION IS THE POINT, and it replaces one that asserted the
|
||||
// opposite. The old test pinned "isFlipt was called with NO entityId/context
|
||||
// (a global eval), so the segment can't match" — which is true and proves
|
||||
// nothing: a global eval returns the flag's BASE value, so that test passed
|
||||
// only because the stub's base was false. See the base-true case below.
|
||||
expect(mockIsFlipt).not.toHaveBeenCalledWith('app-blocks-enabled');
|
||||
expect(mockIsFlipt.mock.calls.filter((c) => c[0] === 'app-blocks-enabled')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('🔴 a vanished subject is STILL refused when the flag is base-`enabled: true` (the GA flip)', async () => {
|
||||
// The forcing condition for this whole gate: `app-blocks-enabled` widened by
|
||||
// BASE rather than by segment. Every stub in this file's default setup has a
|
||||
// false base, which is what made the retracted "global eval → fail-closed"
|
||||
// derivation look tested. Here the flag says yes to everything, exactly as a
|
||||
// base-enabled flag does for a no-entityId eval (measured against the real
|
||||
// wasm engine in `app-blocks-flag.base-enabled-flip.test.ts`).
|
||||
mockIsFlipt.mockImplementation(async () => true);
|
||||
mockGetSessionUser.mockResolvedValue(undefined as never);
|
||||
|
||||
const caller = blocksRouter.createCaller(fakeCtx() as never);
|
||||
await expect(
|
||||
caller.pollWorkflow({ blockToken: 'tok', workflowId: 'wf_1' })
|
||||
).rejects.toMatchObject({
|
||||
code: 'UNAUTHORIZED',
|
||||
message: 'runtime block token subject could not be resolved',
|
||||
});
|
||||
});
|
||||
|
||||
it('POSITIVE CONTROL: the same base-true flag still ADMITS a subject that hydrates', async () => {
|
||||
// Without this, the two refusals above are indistinguishable from a harness
|
||||
// that rejects `pollWorkflow` for some unrelated reason.
|
||||
mockIsFlipt.mockImplementation(async () => true);
|
||||
mockGetSessionUser.mockResolvedValue({ id: 42, isModerator: true, tier: 'gold' } as never);
|
||||
|
||||
const caller = blocksRouter.createCaller(fakeCtx() as never);
|
||||
await expect(
|
||||
caller.pollWorkflow({ blockToken: 'tok', workflowId: 'wf_1' })
|
||||
).resolves.toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('assertViewerIsAppDeveloper — the AUTHOR gate refuses an unresolvable subject', () => {
|
||||
it('🔴 refuses with its OWN message when the subject vanishes between the two gates, base-true flag', async () => {
|
||||
// `updateUserSettings` is the single remaining call site of the author gate. It
|
||||
// runs `assertAppBlocksEnabledForTokenUser` first and `assertViewerIsAppDeveloper`
|
||||
// second, and each resolves the subject independently against the hub-backed
|
||||
// session client — so a subject deleted between the two awaits is a real, if
|
||||
// narrow, state. `mockResolvedValueOnce` reproduces it and is the only way to
|
||||
// reach the author gate's own branch without stubbing the gate under test.
|
||||
mockIsFlipt.mockImplementation(async () => true); // base-`enabled: true`
|
||||
mockGetSessionUser
|
||||
.mockResolvedValueOnce({ id: 42, isModerator: false, tier: 'free' } as never)
|
||||
.mockResolvedValue(undefined as never);
|
||||
|
||||
const caller = blocksRouter.createCaller(fakeCtx() as never);
|
||||
await expect(
|
||||
caller.updateUserSettings({ blockToken: 'tok', settings: {} })
|
||||
).rejects.toMatchObject({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'app-authoring subject could not be resolved',
|
||||
});
|
||||
});
|
||||
|
||||
it('POSITIVE CONTROL: a subject that hydrates on both calls passes BOTH gates', async () => {
|
||||
// Proves the refusal above is attributable to the missing subject, not to the
|
||||
// author capability or to anything downstream: same flag, same input, same
|
||||
// mocks — only the second hydration differs. Asserting the SPECIFIC downstream
|
||||
// outcome is load-bearing: `rejects.not.toMatchObject(...)` passes on ANY other
|
||||
// rejection, so it cannot tell "got past both gates" from "blew up differently".
|
||||
// NOT_FOUND / 'Block install not found' comes from the mocked BlockRegistry
|
||||
// returning no instance, which is several steps PAST both gates.
|
||||
mockIsFlipt.mockImplementation(async () => true);
|
||||
mockGetSessionUser.mockResolvedValue({ id: 42, isModerator: false, tier: 'free' } as never);
|
||||
|
||||
const caller = blocksRouter.createCaller(fakeCtx() as never);
|
||||
await expect(
|
||||
caller.updateUserSettings({ blockToken: 'tok', settings: {} })
|
||||
).rejects.toMatchObject({ code: 'NOT_FOUND', message: 'Block install not found' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -64,6 +64,15 @@ import type { SessionUser } from '~/types/session';
|
||||
*/
|
||||
|
||||
const enforceAppBlocksAuthorFlag = middleware(async ({ ctx, next }) => {
|
||||
// Every attachment below is `protectedProcedure.use(...)`, so `isAuthed` has already
|
||||
// run and `ctx.user` is non-null at runtime — but this is a bare `middleware`, whose
|
||||
// ctx type does not carry that narrowing. `isAppBlocksAuthorEnabled` REQUIRES a
|
||||
// subject (a capability has nothing to authorize without one), so the compiler makes
|
||||
// this refusal mandatory rather than optional. Keep it: it is also what makes the
|
||||
// middleware safe if it is ever attached to a `publicProcedure`.
|
||||
if (!ctx.user) {
|
||||
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'Apps authoring is not enabled' });
|
||||
}
|
||||
if (await isAppBlocksAuthorEnabled({ user: ctx.user })) return next();
|
||||
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'Apps authoring is not enabled' });
|
||||
});
|
||||
|
||||
@@ -119,6 +119,15 @@ const enforceAppBlocksFlag = middleware(async ({ ctx, next }) => {
|
||||
* mods only. (The mod-only `backfillAssets` proc keeps `enforceAppBlocksFlag`.)
|
||||
*/
|
||||
const enforceAppBlocksAuthorFlag = middleware(async ({ ctx, next }) => {
|
||||
// Every attachment below is `protectedProcedure.use(...)`, so `isAuthed` has already
|
||||
// run and `ctx.user` is non-null at runtime — but this is a bare `middleware`, whose
|
||||
// ctx type does not carry that narrowing. `isAppBlocksAuthorEnabled` REQUIRES a
|
||||
// subject (a capability has nothing to authorize without one), so the compiler makes
|
||||
// this refusal mandatory rather than optional. Keep it: it is also what makes the
|
||||
// middleware safe if it is ever attached to a `publicProcedure`.
|
||||
if (!ctx.user) {
|
||||
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'Apps authoring is not enabled' });
|
||||
}
|
||||
if (await isAppBlocksAuthorEnabled({ user: ctx.user })) return next();
|
||||
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'Apps authoring is not enabled' });
|
||||
});
|
||||
|
||||
@@ -174,7 +174,10 @@ interface SharedContext {
|
||||
* 4. for WRITE ops: authenticated subject + the min-trust gate
|
||||
* Anon may READ list/counts; anon NEVER writes/votes.
|
||||
*/
|
||||
export async function resolveSharedContext(blockToken: string, op: SharedOp): Promise<SharedContext> {
|
||||
export async function resolveSharedContext(
|
||||
blockToken: string,
|
||||
op: SharedOp
|
||||
): Promise<SharedContext> {
|
||||
const claims = await verifyBlockToken(blockToken);
|
||||
if (!claims) throw new TRPCError({ code: 'UNAUTHORIZED', message: 'invalid block token' });
|
||||
|
||||
@@ -221,15 +224,44 @@ export async function resolveSharedContext(blockToken: string, op: SharedOp): Pr
|
||||
throw new TRPCError({ code: 'FORBIDDEN', message: 'invalid token subject' });
|
||||
}
|
||||
// Hydrate the TOKEN SUBJECT (block-token path has no ctx.user) — needed for both
|
||||
// the flag segment eval and the trust gate. Fail-closed on a vanished subject.
|
||||
// the flag segment eval and the trust gate.
|
||||
const subjectUser =
|
||||
userId != null
|
||||
? ((await sessionClient.getSessionUserById(userId)) as SessionUser | null)
|
||||
: null;
|
||||
|
||||
// Dedicated fail-closed kill-switch (evaluated with the subject's context so the
|
||||
// flag's mod/cohort segments resolve identically to the client gate; anon read →
|
||||
// global eval → fail-closed until a base-enabled GA flip).
|
||||
// 🔴 A VANISHED SUBJECT IS NOT AN ANONYMOUS CALLER — refuse it here, before the
|
||||
// flag. Both used to collapse into the single `subjectUser ?? undefined` below,
|
||||
// and the comment claimed that was "fail-closed on a vanished subject". It was
|
||||
// not: a no-user eval cannot match a segment, but its answer is the flag's own
|
||||
// base `enabled` value, so a base-`enabled: true` GA flip of
|
||||
// `app-blocks-shared-storage` would admit a token whose subject no longer exists.
|
||||
// The WRITE path happened to catch it downstream (the `userId == null` check +
|
||||
// the min-trust gate below); EVERY op in `READ_OPS` skips that block entirely and
|
||||
// has no second belt, so the flag was the only thing standing in front of all of
|
||||
// them. Do not re-enumerate that set here — it is four ops today and adding a
|
||||
// fifth must not silently make this comment wrong. Mechanism + the measurement
|
||||
// against the real wasm engine: GLOBAL-EVAL SEMANTICS in `app-blocks-flag.ts`.
|
||||
//
|
||||
// 🔴 WATCHLISTED as `shared-storage-subject-refusal` in
|
||||
// `scripts/compiled-branch-watchlist.mjs`. Unlike a type-level guard, this is a pure
|
||||
// runtime branch, so a bundler that drops it re-opens the exposure with the source
|
||||
// still correct — which is precisely what shipped in release 5.1.18 (civitai#3983).
|
||||
// MOVING this branch is fine — the gate resolves its anchor from source at run time,
|
||||
// so line numbers do not matter, and the message text is free to change because the
|
||||
// anchor here is the CONDITION on the next line, not the message. DELETING the branch,
|
||||
// or rewriting that condition, fails the production Docker build at
|
||||
// `assert-compiled-branches.mjs` — in the second case update the watchlist entry in the
|
||||
// same commit.
|
||||
if (userId != null && !subjectUser) {
|
||||
throw new TRPCError({ code: 'FORBIDDEN', message: 'token subject could not be resolved' });
|
||||
}
|
||||
|
||||
// Dedicated kill-switch, evaluated with the subject's context so the flag's
|
||||
// mod/cohort segments resolve identically to the client gate. An ANON token
|
||||
// (`sub:'anon'`, `userId == null`) still reaches this with no user, which is
|
||||
// deliberate: that is a global eval, i.e. the flag's BASE value, and anon shared
|
||||
// access is exactly the GA widening a base-`enabled` flip is meant to perform.
|
||||
if (!(await isAppBlocksSharedStorageEnabled({ user: subjectUser ?? undefined }))) {
|
||||
throw new TRPCError({ code: 'FORBIDDEN', message: 'shared storage is not enabled' });
|
||||
}
|
||||
@@ -315,9 +347,7 @@ export const appsSharedRouter = router({
|
||||
const { schema, userId } = await resolveSharedContext(input.blockToken, 'list');
|
||||
const pool = requireAppsDb();
|
||||
|
||||
const afterKey = input.cursor
|
||||
? Buffer.from(input.cursor, 'base64').toString('utf8')
|
||||
: null;
|
||||
const afterKey = input.cursor ? Buffer.from(input.cursor, 'base64').toString('utf8') : null;
|
||||
const escapedPrefix = (input.prefix ?? '').replace(/([\\%_])/g, '\\$1');
|
||||
const prefixPattern = `${escapedPrefix}%`;
|
||||
|
||||
@@ -709,10 +739,9 @@ export const appsSharedRouter = router({
|
||||
// Visibility/existence pre-check → NOT_FOUND for hidden OR missing (H2). The
|
||||
// FK on votes.key is the belt for a race between this and the insert.
|
||||
const exists = (
|
||||
await pool.query(
|
||||
`SELECT 1 FROM ${schema}.shared_kv WHERE key = $1 AND hidden_at IS NULL`,
|
||||
[input.key]
|
||||
)
|
||||
await pool.query(`SELECT 1 FROM ${schema}.shared_kv WHERE key = $1 AND hidden_at IS NULL`, [
|
||||
input.key,
|
||||
])
|
||||
).rowCount;
|
||||
if (!exists) throw new TRPCError({ code: 'NOT_FOUND', message: 'request not found' });
|
||||
|
||||
@@ -816,9 +845,7 @@ export const appsSharedRouter = router({
|
||||
* spam). Does not hide the row — a moderator decides via `apps.mod.purgeSharedRow`.
|
||||
*/
|
||||
report: publicProcedure
|
||||
.input(
|
||||
blockTokenInput.extend({ key: sharedKeyInput, reason: z.string().max(500).optional() })
|
||||
)
|
||||
.input(blockTokenInput.extend({ key: sharedKeyInput, reason: z.string().max(500).optional() }))
|
||||
.mutation(async ({ input }) => {
|
||||
const { userId, slug, schema, appBlockId } = await resolveSharedContext(
|
||||
input.blockToken,
|
||||
|
||||
@@ -297,9 +297,35 @@ const enforceAppBlocksFlag = middleware(async ({ ctx, next, type }) => {
|
||||
* enabled kill-switch that runs right before this) — `sessionClient
|
||||
* .getSessionUserById`, the authoritative hub-backed resolver, never a
|
||||
* client-supplied value — so `buildFliptContext` sees the subject's real
|
||||
* isModerator/tier and the mod floor / segment match can't be spoofed. A
|
||||
* vanished user → undefined → no mod floor + global eval (never matches a
|
||||
* segment) → FORBIDDEN (fail-closed).
|
||||
* isModerator/tier and the mod floor / segment match can't be spoofed.
|
||||
*
|
||||
* 🔴 A VANISHED SUBJECT IS REFUSED BEFORE THE CAPABILITY IS EVALUATED. This
|
||||
* docblock used to say "a vanished user → undefined → no mod floor + global eval
|
||||
* (never matches a segment) → FORBIDDEN (fail-closed)", and that derivation was
|
||||
* wrong: a no-user eval cannot match a segment, but its answer is the flag's own
|
||||
* base `enabled` value, so a base-`enabled: true` widening of
|
||||
* `app-blocks-author` would have turned an unresolvable subject into a PASS on an
|
||||
* AUTHZ gate. The refusal is now structural — no subject, no capability, no Flipt
|
||||
* call — which is the same shape `apps.router.ts` already uses for its own
|
||||
* `assertViewerIsAppDeveloper`. It is not optional politeness: `user` is a
|
||||
* REQUIRED, non-nullable parameter of `isAppBlocksAuthorEnabled`, so this narrowing
|
||||
* is what makes the next line compile, and deleting it is a type error rather than
|
||||
* a silent re-opening. Mechanism + the measurement: see GLOBAL-EVAL SEMANTICS in
|
||||
* `app-blocks-flag.ts`.
|
||||
*
|
||||
* 🔴 Unlike its sibling `assertAppBlocksEnabledForTokenUser`, this refusal is NOT on
|
||||
* the compiled-branch watchlist, and that is measured rather than assumed: losing it
|
||||
* cannot silently re-open anything, because `isAppBlocksAuthorEnabled` takes a
|
||||
* non-nullable subject and dereferences it immediately, so a dropped guard yields a
|
||||
* `TypeError` (a 500) rather than a pass. The enabled gate's guard IS watchlisted,
|
||||
* because losing THAT one falls through to a global eval returning the flag's base
|
||||
* value. Its message text differs from this one's on purpose — two different
|
||||
* conditions, and an identical string under a different code is not separable in a
|
||||
* log. Both are also distinct APP-WIDE, which is the level that actually matters to
|
||||
* an operator: the kill-switch one was byte-identical to `apps.router.ts`'s
|
||||
* structurally-identical refusal until it was renamed to `'runtime block token
|
||||
* subject could not be resolved'`. If you add a fourth refusal of this shape, give
|
||||
* it text no other one uses — and note that this one doubles as a watchlist anchor.
|
||||
*
|
||||
* This is the AUTHZ half only; the `isAppBlocksEnabled` kill-switch
|
||||
* (`assertAppBlocksEnabledForTokenUser`) still runs first and is unchanged — it
|
||||
@@ -307,7 +333,13 @@ const enforceAppBlocksFlag = middleware(async ({ ctx, next, type }) => {
|
||||
*/
|
||||
async function assertViewerIsAppDeveloper(userId: number): Promise<void> {
|
||||
const user = (await sessionClient.getSessionUserById(userId)) as SessionUser | null;
|
||||
if (!(await isAppBlocksAuthorEnabled({ user: user ?? undefined }))) {
|
||||
if (!user) {
|
||||
throw new TRPCError({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'app-authoring subject could not be resolved',
|
||||
});
|
||||
}
|
||||
if (!(await isAppBlocksAuthorEnabled({ user }))) {
|
||||
throw new TRPCError({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'Apps authoring is not enabled for this account',
|
||||
@@ -353,10 +385,24 @@ async function assertAppEditAccess(
|
||||
* user, not `ctx.user`.
|
||||
*
|
||||
* The flag stays a real kill-switch (a flip still shuts these procs down) — we
|
||||
* only fix the IDENTITY it's evaluated against. This does NOT widen access: the
|
||||
* mod-segmented flag resolves `true` only for a moderator subject; a non-mod or
|
||||
* anon (`sub:'anon'` → no resolvable user) subject still resolves `false` →
|
||||
* blocked. `authorizeBlockBridgeToken` (caller) already rejected invalid/expired
|
||||
* only fix the IDENTITY it's evaluated against. This does NOT widen access: with
|
||||
* the flag base-`false` + `moderators`/cohort segments as it is today, it resolves
|
||||
* `true` only for an in-segment subject and a non-mod outside the cohort resolves
|
||||
* `false` → blocked. An ANONYMOUS token (`sub:'anon'`) never reaches this function
|
||||
* at all — each of its 16 call sites runs `parseSubjectUserId(claims.sub)` and
|
||||
* throws UNAUTHORIZED on `null` first, so the no-subject case handled below is a
|
||||
* VANISHED user, not an anon caller. There are **16** such parse sites, not 17:
|
||||
* the 17th gate call is `assertViewerIsAppDeveloper`, which shares the parse site
|
||||
* of the enabled-gate call immediately above it rather than adding one, so the two
|
||||
* sets OVERLAP and must not be added. (No raw-occurrence total is recorded here,
|
||||
* and none should be: a grep for either identifier also matches this docblock's
|
||||
* own prose and the import at the top of the file, and for the gate it matches a
|
||||
* DIFFERENT function of the same name in `apps.router.ts`. Nor is a re-derivation
|
||||
* command given — the obvious one contains the identifier it searches for, so it
|
||||
* matches the very line it is written on and returns one too many. Enumerate the
|
||||
* call sites if you need the number; this paragraph has now been wrong four
|
||||
* rounds running, each time by writing a figure down.)
|
||||
* `authorizeBlockBridgeToken` (caller) already rejected invalid/expired
|
||||
* tokens, revoked instances and non-approved apps before this runs — the "revoked"
|
||||
* half of that sentence used to be false, because the caller ran a bare
|
||||
* `verifyBlockToken`, which never checked it. Every other belt (the per-scope
|
||||
@@ -385,15 +431,42 @@ async function assertAppEditAccess(
|
||||
*/
|
||||
async function assertAppBlocksEnabledForTokenUser(userId: number): Promise<void> {
|
||||
// Full, authoritative SessionUser (cached; tier derived from active
|
||||
// subscriptions) so buildFliptContext sees the user's REAL tier/isMember,
|
||||
// not type-defaults. A vanished user → undefined → global eval → flag false
|
||||
// → blocked (fail-closed). This is the LAST identity-shaped belt on most runtime
|
||||
// procs now that the author gate is off them, so its fail-closed posture is not
|
||||
// backed up by a second one — do not weaken it.
|
||||
// getSessionUserById returns the package SessionUser (loosely typed at this boundary — cast as bearer-token.ts
|
||||
// does) or null for a vanished user. null → undefined → isAppBlocksEnabled's global eval → flag false → blocked.
|
||||
// subscriptions) so buildFliptContext sees the user's REAL tier/isMember, not
|
||||
// type-defaults. getSessionUserById returns the package SessionUser (loosely
|
||||
// typed at this boundary — cast as bearer-token.ts does) or null for a vanished
|
||||
// user. This is the LAST identity-shaped belt on most runtime procs now that the
|
||||
// author gate is off them, so its fail-closed posture is not backed up by a
|
||||
// second one — do not weaken it.
|
||||
const user = (await sessionClient.getSessionUserById(userId)) as SessionUser | null;
|
||||
if (!(await isAppBlocksEnabled({ user: user ?? undefined }))) {
|
||||
// 🔴 REFUSE AN UNHYDRATABLE SUBJECT OUTRIGHT, before the flag is consulted.
|
||||
// This used to pass `{ user: user ?? undefined }`, and the comment derived the
|
||||
// denial from "global eval → flag false → blocked". The premise holds (a no-user
|
||||
// eval carries entityId 'global' and an empty context, which no segment can
|
||||
// match) but the conclusion came from `app-blocks-enabled` being base-`false`,
|
||||
// not from the segment miss: a global eval returns the flag's own base value, so
|
||||
// a base-`enabled: true` GA flip would have let a token whose subject no longer
|
||||
// resolves through this gate. `isAppBlocksEnabled`'s no-user branch is KEPT for
|
||||
// its real machine caller, so the refusal has to live here. Mechanism + the
|
||||
// measurement against the real wasm engine: GLOBAL-EVAL SEMANTICS in
|
||||
// `app-blocks-flag.ts`. Distinct message so the two refusals stay separable.
|
||||
//
|
||||
// 🔴 WATCHLISTED as `block-token-subject-refusal` in
|
||||
// `scripts/compiled-branch-watchlist.mjs`. Unlike a type-level guard, this is a pure
|
||||
// runtime branch, so a bundler that drops it re-opens the exposure with the source
|
||||
// still correct — which is precisely what shipped in release 5.1.18 (civitai#3983).
|
||||
// MOVING this branch is fine — the gate resolves its anchor from source at run time,
|
||||
// so line numbers do not matter. DELETING it fails the production Docker build at
|
||||
// `assert-compiled-branches.mjs`. And 🔴 REWORDING THE MESSAGE BELOW IS A WATCHLIST
|
||||
// EDIT: that exact string IS this entry's anchor, so changing it makes the gate exit 2
|
||||
// ("no line contains this anchor") — a failure that reads like gate breakage rather
|
||||
// than like the copy change that caused it. Update the entry in the same commit.
|
||||
if (!user) {
|
||||
throw new TRPCError({
|
||||
code: 'UNAUTHORIZED',
|
||||
message: 'runtime block token subject could not be resolved',
|
||||
});
|
||||
}
|
||||
if (!(await isAppBlocksEnabled({ user }))) {
|
||||
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'Apps are not enabled' });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import sourceSnapshot from './fixtures/flipt-store-scope.snapshot.json';
|
||||
import {
|
||||
deriveSnapshotFromFlagShape,
|
||||
startFliptFixtureServer,
|
||||
SNAPSHOT_PATH,
|
||||
type FliptFixtureServer,
|
||||
} from './fixtures/flipt-fixture-server';
|
||||
import type { SessionUser } from '~/types/session';
|
||||
|
||||
/**
|
||||
* 🔴 THE MEASUREMENT BEHIND "GLOBAL-EVAL SEMANTICS" IN `app-blocks-flag.ts`.
|
||||
*
|
||||
* Several docblocks in that file used to reason:
|
||||
*
|
||||
* no user → a global eval that can never match a segment → fail-closed / denied
|
||||
*
|
||||
* The premise is true. The conclusion does not follow from it — it follows from the
|
||||
* flag's BASE `enabled` value being `false`. When no rollout matches, Flipt answers
|
||||
* with the flag's own base value, so a base-`enabled: true` widening turns every
|
||||
* no-user branch in that file from a deny into a pass.
|
||||
*
|
||||
* ## This is a reproduction, not a discovery
|
||||
*
|
||||
* The inversion was already measured against PRODUCTION Flipt (v2.10.0, 2026-08-15)
|
||||
* and is recorded in `civitai/flipt-state`'s `scripts/validate-flag-shape.py`
|
||||
* docstring, with both controls firing. That validator exists to guard this exact
|
||||
* shape. What it CANNOT cover is the code side: it blocks the misconfiguration form
|
||||
* (a boolean flag whose rollouts are all segment-scoped must be base-false) but
|
||||
* deliberately exempts flags with no rollouts at all — which is precisely the shape
|
||||
* of an intended GA flip. So the config gate permits the GA by design, and the code
|
||||
* is the only control left on that half. This suite is that control's evidence.
|
||||
*
|
||||
* Nothing in THIS repo measured it, which is how the inference survived here: the
|
||||
* sibling suites stub `isFlipt` with a fake whose base is `false`, so they reproduce
|
||||
* the conclusion without ever exercising its precondition.
|
||||
*
|
||||
* derived snapshot → a real HTTP server on localhost
|
||||
* → the REAL `createFliptClient` from `@civitai/flipt`
|
||||
* → the REAL `@flipt-io/flipt-client-js` wasm engine
|
||||
* → the REAL `isAppBlocksAuthorEnabled` / `isAppBlocksEnabled`
|
||||
*
|
||||
* ## About the fixture
|
||||
*
|
||||
* DERIVED at runtime from `fixtures/flipt-store-scope.snapshot.json` — a real Flipt
|
||||
* v2 evaluation snapshot carrying the production flag SHAPE (base + a single
|
||||
* `SEGMENT_ROLLOUT_TYPE` whose `OR_SEGMENT_OPERATOR` combines an
|
||||
* `ALL_SEGMENT_MATCH_TYPE` moderator segment with an `ANY_SEGMENT_MATCH_TYPE`
|
||||
* allowlist). Only the key and `enabled` change. It is deliberately NOT a second
|
||||
* checked-in file: that source's own docblock notes a re-capture means re-anonymising
|
||||
* it, so a hand-edited twin would silently keep the old segment shape while still
|
||||
* claiming production fidelity. `base-false-control` is the same shape left at
|
||||
* `enabled: false`, so every `true` below is attributable to the base value and not
|
||||
* to the harness.
|
||||
*
|
||||
* ## What this suite structurally CANNOT see
|
||||
*
|
||||
* - The live production flag documents. Both flags are base `false` with segment
|
||||
* rollouts TODAY, so this fixture is a hypothetical, deliberately: the point is
|
||||
* that the code must not depend on that staying true.
|
||||
* - The real network path to production Flipt (TLS, auth, circuit breaker, refreshes).
|
||||
* - `FLIPT_LOCAL_OVERRIDES`, the other route to a no-user `true`. It is hard-disabled
|
||||
* when `NODE_ENV === 'production'` (`packages/civitai-flipt/src/env.ts`).
|
||||
* - The 2 of 10 call sites that hand `isAppBlocksAuthorEnabled` a nullable subject.
|
||||
* Those are a COMPILE error now, not a runtime one, so the guard for them is
|
||||
* `pnpm typecheck` and there is deliberately no test here pretending otherwise.
|
||||
*/
|
||||
|
||||
vi.hoisted(() => {
|
||||
process.env.SERVER_DOMAIN_GREEN = 'civitai.com';
|
||||
process.env.SERVER_DOMAIN_BLUE = 'civitai.blue';
|
||||
process.env.SERVER_DOMAIN_RED = 'civitai.red';
|
||||
});
|
||||
|
||||
const URL_ENV = '__TEST_FLIPT_BASE_FLIP_URL';
|
||||
|
||||
vi.mock('~/server/flipt/client', async () => {
|
||||
const { buildRealFliptClientMock } = await import('./fixtures/flipt-fixture-server');
|
||||
return buildRealFliptClientMock('__TEST_FLIPT_BASE_FLIP_URL');
|
||||
});
|
||||
|
||||
/**
|
||||
* The GA-flip hypothetical: `app-blocks-author` and `app-blocks-enabled` widened by
|
||||
* BASE while still carrying their segment rollout, plus the same shape left base-false
|
||||
* as the negative control.
|
||||
*/
|
||||
const baseTrueSnapshot = deriveSnapshotFromFlagShape(
|
||||
sourceSnapshot as never,
|
||||
'app-blocks-enabled',
|
||||
[
|
||||
{ key: 'app-blocks-author', enabled: true },
|
||||
{ key: 'app-blocks-enabled', enabled: true },
|
||||
{ key: 'base-false-control', enabled: false },
|
||||
]
|
||||
);
|
||||
|
||||
let server: FliptFixtureServer;
|
||||
|
||||
beforeAll(async () => {
|
||||
server = await startFliptFixtureServer(baseTrueSnapshot);
|
||||
process.env[URL_ENV] = server.url;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await server.close();
|
||||
});
|
||||
|
||||
/** Minimal SessionUser — only the fields `buildFliptContext` reads. */
|
||||
function sessionUser(id: number, extra: Partial<SessionUser> = {}): SessionUser {
|
||||
return { id, isModerator: false, tier: 'free', onboarding: 0, ...extra } as SessionUser;
|
||||
}
|
||||
|
||||
const UNAFFILIATED_ID = 4242; // matches no segment in the derived snapshot
|
||||
|
||||
describe('a base-`enabled: true` flip, measured against the real Flipt engine', () => {
|
||||
it('INSTRUMENT CONTROL: the fixture server is reached, and an unknown key still fails closed', async () => {
|
||||
const { isFlipt } = await import('~/server/flipt/client');
|
||||
await expect(isFlipt('a-flag-that-does-not-exist')).resolves.toBe(false);
|
||||
expect(server.received.length).toBeGreaterThan(0);
|
||||
expect(server.received[0].url).toContain(SNAPSHOT_PATH);
|
||||
expect(server.received[0].environment).toBe('civitai-app');
|
||||
expect(server.received[0].auth).toBe('Bearer test-token');
|
||||
});
|
||||
|
||||
it('FIXTURE CONTROL: the derived flags really carry a SEGMENT rollout (not a bare boolean, not a threshold)', async () => {
|
||||
// Without this, every `true` below could come from a flag with no rollouts at all,
|
||||
// or from a THRESHOLD rollout — both different shapes making a different claim, and
|
||||
// a 100% threshold would also make `base-false-control` true, so the negative
|
||||
// control would not catch the swap either. Assert the TYPE, which is what the
|
||||
// title claims; a length check does not.
|
||||
for (const flag of baseTrueSnapshot.flags) {
|
||||
const rollouts = (flag as { rollouts?: { type?: string }[] }).rollouts;
|
||||
expect(Array.isArray(rollouts)).toBe(true);
|
||||
expect(rollouts?.map((r) => r.type)).toContain('SEGMENT_ROLLOUT_TYPE');
|
||||
}
|
||||
});
|
||||
|
||||
it('FIXTURE-GUARD CONTROL: the derivation helper REFUSES every template shape that is not a clean segment rollout', async () => {
|
||||
// Makes the guard reachable rather than merely present — the four refusal arms
|
||||
// are otherwise exercised by nothing, which is the shape this whole PR is about.
|
||||
// Each arm carries its OWN expected message, so an arm cannot pass by tripping a
|
||||
// DIFFERENT arm's error and read as covered.
|
||||
const { deriveSnapshotFromFlagShape: derive } = await import('./fixtures/flipt-fixture-server');
|
||||
const ask = [{ key: 'x', enabled: true }];
|
||||
const NO_SEGMENT = /carries no SEGMENT_ROLLOUT_TYPE rollout/;
|
||||
const shapes: Array<[string, unknown, RegExp]> = [
|
||||
['no rollouts key', { key: 't', enabled: false }, NO_SEGMENT],
|
||||
['empty rollouts', { key: 't', enabled: false, rollouts: [] }, NO_SEGMENT],
|
||||
[
|
||||
'threshold rollout only',
|
||||
{ key: 't', enabled: false, rollouts: [{ type: 'THRESHOLD_ROLLOUT_TYPE' }] },
|
||||
NO_SEGMENT,
|
||||
],
|
||||
// 🔴 THE ARM `.some()` ALONE ADMITTED, and the reason this guard was widened: a
|
||||
// percentage ramp captured ALONGSIDE the segment rollout. The segment rollout IS
|
||||
// present, so the three arms above cannot catch it — and a 100% threshold makes
|
||||
// `base-false-control` evaluate true for every subject, which is exactly the
|
||||
// attribution this fixture exists to protect.
|
||||
[
|
||||
'threshold rollout ALONGSIDE the segment rollout',
|
||||
{
|
||||
key: 't',
|
||||
enabled: false,
|
||||
rollouts: [{ type: 'THRESHOLD_ROLLOUT_TYPE' }, { type: 'SEGMENT_ROLLOUT_TYPE' }],
|
||||
},
|
||||
/carries a THRESHOLD_ROLLOUT_TYPE rollout alongside its segment rollout/,
|
||||
],
|
||||
];
|
||||
for (const [label, flag, expected] of shapes) {
|
||||
expect(() =>
|
||||
derive({ namespace: { key: 'default' }, flags: [flag] } as never, 't', ask)
|
||||
).toThrow(expected);
|
||||
expect(label).toBeTruthy();
|
||||
}
|
||||
// POSITIVE CONTROL — the real template is accepted, so the three refusals above
|
||||
// are attributable to the shape and not to the helper rejecting everything.
|
||||
expect(() => derive(sourceSnapshot as never, 'app-blocks-enabled', ask)).not.toThrow();
|
||||
});
|
||||
|
||||
it('🔴 THE RETRACTED PREMISE, MEASURED: a GLOBAL eval of a base-true segmented flag returns TRUE', async () => {
|
||||
const { isFlipt } = await import('~/server/flipt/client');
|
||||
// No entityId, no context — the exact call every no-user branch in
|
||||
// app-blocks-flag.ts makes. The segment genuinely cannot match; the answer is
|
||||
// the flag's BASE value, and here that is `true`.
|
||||
await expect(isFlipt('app-blocks-author')).resolves.toBe(true);
|
||||
await expect(isFlipt('app-blocks-enabled')).resolves.toBe(true);
|
||||
// NEGATIVE CONTROL, same shape and the same rollout, base `false` → `false`.
|
||||
// So the two `true`s above are attributable to the base value, not the harness.
|
||||
await expect(isFlipt('base-false-control')).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('the segment still cannot match a global eval — the premise was right, only the conclusion was not', async () => {
|
||||
const { isFlipt } = await import('~/server/flipt/client');
|
||||
const { buildFliptContext } = await import('~/server/services/feature-flags.service');
|
||||
// `base-false-control` carries the moderator segment. WITH a moderator context it
|
||||
// matches and resolves true; with the global (no-context) eval above it did not.
|
||||
const mod = sessionUser(777, { isModerator: true });
|
||||
await expect(
|
||||
isFlipt('base-false-control', String(mod.id), buildFliptContext(mod))
|
||||
).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('the author gate evaluates a real subject against the base-true flag (the path that still exists)', async () => {
|
||||
const { isAppBlocksAuthorEnabled } = await import('~/server/services/app-blocks-flag');
|
||||
// `isAppBlocksAuthorEnabled` has no no-user branch left to test at runtime — its
|
||||
// `user` parameter is required and non-nullable, so the undefined case is a
|
||||
// COMPILE error. What remains testable is that a present subject is still
|
||||
// evaluated normally under the same flag, i.e. the type change did not turn the
|
||||
// helper into a blanket deny.
|
||||
await expect(isAppBlocksAuthorEnabled({ user: sessionUser(UNAFFILIATED_ID) })).resolves.toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it('isAppBlocksEnabled KEEPS its no-user global eval — deliberately, because it is a kill-switch', async () => {
|
||||
const { isAppBlocksEnabled } = await import('~/server/services/app-blocks-flag');
|
||||
// The asymmetry with the author helper, pinned so nobody "unifies" them. A
|
||||
// kill-switch answers "is the feature on at all", which a subject-less machine
|
||||
// path may legitimately ask and which the base value IS. A capability answers
|
||||
// "may THIS subject", which is unanswerable without one.
|
||||
await expect(isAppBlocksEnabled()).resolves.toBe(true);
|
||||
});
|
||||
});
|
||||
+20
-53
@@ -1,7 +1,10 @@
|
||||
import { createServer, type Server } from 'http';
|
||||
import type { AddressInfo } from 'net';
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import snapshot from './fixtures/flipt-store-scope.snapshot.json';
|
||||
import {
|
||||
startFliptFixtureServer,
|
||||
SNAPSHOT_PATH,
|
||||
type FliptFixtureServer,
|
||||
} from './fixtures/flipt-fixture-server';
|
||||
import type { SessionUser } from '~/types/session';
|
||||
|
||||
/**
|
||||
@@ -55,62 +58,26 @@ vi.hoisted(() => {
|
||||
process.env.SERVER_DOMAIN_RED = 'civitai.red';
|
||||
});
|
||||
|
||||
const SNAPSHOT_PATH = '/internal/v1/evaluation/snapshot/namespace/default';
|
||||
|
||||
/** Requests the fake Flipt actually received — used as the positive control. */
|
||||
const received: { url: string; environment?: string; auth?: string }[] = [];
|
||||
|
||||
let server: Server;
|
||||
let baseUrl = '';
|
||||
|
||||
/**
|
||||
* Substitutes ONLY the app's env plumbing (`~/env/server` is not loadable in a unit
|
||||
* run): the exported `isFlipt` is a REAL `createFliptClient` instance pointed at the
|
||||
* fixture server. Everything the defect could live in — the client factory, its cache,
|
||||
* the wasm engine, the segment matcher — is the production code.
|
||||
* The server + client-mock plumbing is shared with
|
||||
* `app-blocks-flag.base-enabled-flip.test.ts` via `fixtures/flipt-fixture-server`.
|
||||
* Only the SNAPSHOT differs between the two suites — this one serves the captured
|
||||
* production shapes (base OFF), that one serves the same shapes re-keyed base ON.
|
||||
*/
|
||||
let server: FliptFixtureServer;
|
||||
|
||||
vi.mock('~/server/flipt/client', async () => {
|
||||
const { createFliptClient } = await import('@civitai/flipt');
|
||||
const flipt = createFliptClient({
|
||||
url: process.env.__TEST_FLIPT_URL as string,
|
||||
clientToken: 'test-token',
|
||||
environment: 'civitai-app',
|
||||
log: () => undefined,
|
||||
onInitError: (e) => {
|
||||
throw e;
|
||||
},
|
||||
});
|
||||
return {
|
||||
isFlipt: flipt.isEnabled,
|
||||
isFliptSync: flipt.isEnabledSync,
|
||||
getFliptVariant: flipt.getVariant,
|
||||
getFliptBoolean: flipt.getBoolean,
|
||||
ensureFliptInitialized: flipt.ensureInitialized,
|
||||
};
|
||||
const { buildRealFliptClientMock } = await import('./fixtures/flipt-fixture-server');
|
||||
return buildRealFliptClientMock('__TEST_FLIPT_URL');
|
||||
});
|
||||
|
||||
beforeAll(async () => {
|
||||
server = createServer((req, res) => {
|
||||
received.push({
|
||||
url: req.url ?? '',
|
||||
environment: req.headers['x-flipt-environment'] as string | undefined,
|
||||
auth: req.headers.authorization as string | undefined,
|
||||
});
|
||||
if (!req.url?.startsWith(SNAPSHOT_PATH)) {
|
||||
res.writeHead(404).end('{}');
|
||||
return;
|
||||
}
|
||||
res.writeHead(200, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify(snapshot));
|
||||
});
|
||||
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
|
||||
const { port } = server.address() as AddressInfo;
|
||||
baseUrl = `http://127.0.0.1:${port}`;
|
||||
process.env.__TEST_FLIPT_URL = baseUrl;
|
||||
server = await startFliptFixtureServer(snapshot);
|
||||
process.env.__TEST_FLIPT_URL = server.url;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await new Promise<void>((resolve) => server.close(() => resolve()));
|
||||
await server.close();
|
||||
});
|
||||
|
||||
/** Minimal SessionUser — only the fields `buildFliptContext` reads. */
|
||||
@@ -130,10 +97,10 @@ describe('resolveStoreVisibilityScope over the REAL Flipt client (civitai#3983)'
|
||||
// A flag key that is NOT in the fixture: proves an unknown flag fails CLOSED
|
||||
// rather than the client answering `true` for everything.
|
||||
await expect(isFlipt('a-flag-that-does-not-exist')).resolves.toBe(false);
|
||||
expect(received.length).toBeGreaterThan(0);
|
||||
expect(received[0].url).toContain(SNAPSHOT_PATH);
|
||||
expect(received[0].environment).toBe('civitai-app');
|
||||
expect(received[0].auth).toBe('Bearer test-token');
|
||||
expect(server.received.length).toBeGreaterThan(0);
|
||||
expect(server.received[0].url).toContain(SNAPSHOT_PATH);
|
||||
expect(server.received[0].environment).toBe('civitai-app');
|
||||
expect(server.received[0].auth).toBe('Bearer test-token');
|
||||
});
|
||||
|
||||
it('POSITIVE CONTROL: the wasm engine really does match a segment (not a suite wired to nothing)', async () => {
|
||||
|
||||
@@ -201,18 +201,52 @@ describe('isAppBlocksAuthorEnabled — author capability (developer soft-launch)
|
||||
await expect(isAppBlocksAuthorEnabled({ user })).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('resolves OFF for an anonymous / vanished user (no floor, global eval never matches)', async () => {
|
||||
await expect(isAppBlocksAuthorEnabled({ user: undefined })).resolves.toBe(false);
|
||||
await expect(isAppBlocksAuthorEnabled()).resolves.toBe(false);
|
||||
expect(mockIsFlipt).toHaveBeenCalledWith('app-blocks-author');
|
||||
/**
|
||||
* 🔴 THE UNDEFINED-USER CASE IS NOT TESTED AT RUNTIME, ON PURPOSE.
|
||||
*
|
||||
* There used to be a test here asserting that an undefined user resolved `false`
|
||||
* because the helper "fell through to a global eval, which can never match a
|
||||
* segment". The premise is true and the conclusion was not — a global eval returns
|
||||
* the flag's BASE value — so that test passed only because this file's fake has a
|
||||
* false base, and would have gone green over a real authz bypass under a base-true
|
||||
* flag.
|
||||
*
|
||||
* `isAppBlocksAuthorEnabled`'s `user` parameter is now REQUIRED and non-nullable, so
|
||||
* the undefined case is a COMPILE error, not a runtime branch. The guard is
|
||||
* `pnpm typecheck` (and CI's `tekton / typecheck` + `App unit tests + typecheck`).
|
||||
* Re-adding a runtime assertion here would need an `as never` cast to compile — i.e.
|
||||
* it could only test a path the type system already forbids, while reading as
|
||||
* coverage.
|
||||
*
|
||||
* The one runtime claim left is the residual the helper's docblock states, and it is
|
||||
* pinned below: a caller who DEFEATS the type with a cast gets a crash, never a pass.
|
||||
*/
|
||||
it('a cast-defeated undefined subject THROWS — it never returns true (the documented residual)', async () => {
|
||||
mockIsFlipt.mockImplementation(async () => true); // base-`enabled: true`
|
||||
await expect(
|
||||
isAppBlocksAuthorEnabled(undefined as unknown as { user: SessionUser })
|
||||
).rejects.toThrow(TypeError);
|
||||
await expect(
|
||||
isAppBlocksAuthorEnabled({ user: undefined } as unknown as { user: SessionUser })
|
||||
).rejects.toThrow(TypeError);
|
||||
// The flag was never consulted, so the throw cannot be mistaken for an eval result.
|
||||
expect(mockIsFlipt).not.toHaveBeenCalled();
|
||||
// POSITIVE CONTROL — same base-true stub, same call, a real subject. Without this
|
||||
// the rejections above are indistinguishable from a helper that is simply broken.
|
||||
await expect(isAppBlocksAuthorEnabled({ user: makeUser({ id: 555 }) })).resolves.toBe(true);
|
||||
expect(mockIsFlipt).toHaveBeenCalledWith(
|
||||
'app-blocks-author',
|
||||
'555',
|
||||
expect.objectContaining({ userId: '555' })
|
||||
);
|
||||
});
|
||||
|
||||
it('Flipt-down / flag absent → mods only (static fallback), non-mods denied', async () => {
|
||||
// isFlipt returns false for everything (flag absent or Flipt unreachable).
|
||||
mockIsFlipt.mockImplementation(async () => false);
|
||||
await expect(
|
||||
isAppBlocksAuthorEnabled({ user: makeUser({ isModerator: true }) })
|
||||
).resolves.toBe(true); // mod floor
|
||||
await expect(isAppBlocksAuthorEnabled({ user: makeUser({ isModerator: true }) })).resolves.toBe(
|
||||
true
|
||||
); // mod floor
|
||||
await expect(
|
||||
isAppBlocksAuthorEnabled({ user: makeUser({ id: 777, isModerator: false }) })
|
||||
).resolves.toBe(false); // cohort denied when flag absent
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
import { createServer, type Server } from 'http';
|
||||
import type { AddressInfo } from 'net';
|
||||
|
||||
/**
|
||||
* Shared harness for the two suites that exercise the REAL Flipt client against a
|
||||
* REAL evaluation snapshot — `app-blocks-flag.real-flipt-client.integration.test.ts`
|
||||
* (production shapes, base OFF) and `app-blocks-flag.base-enabled-flip.test.ts`
|
||||
* (the same shapes re-keyed with base ON).
|
||||
*
|
||||
* It exists because those two suites were byte-for-byte duplicating the server, the
|
||||
* env plumbing and the request ledger. The instrument is the same in both; only the
|
||||
* snapshot served differs. A snapshot cannot be shared between them — the base-false
|
||||
* and base-true cases assign opposite values to `app-blocks-enabled` — so the
|
||||
* SERVER is the shared part, not the fixture.
|
||||
*/
|
||||
|
||||
/** The path the Flipt v2 client fetches its evaluation snapshot from. */
|
||||
export const SNAPSHOT_PATH = '/internal/v1/evaluation/snapshot/namespace/default';
|
||||
|
||||
export type FliptFixtureServer = {
|
||||
/** Base URL the client should be pointed at. */
|
||||
url: string;
|
||||
/** Every request the fake Flipt received — the instrument control. */
|
||||
received: { url: string; environment?: string; auth?: string }[];
|
||||
close: () => Promise<void>;
|
||||
};
|
||||
|
||||
/**
|
||||
* Start a localhost server that serves `snapshot` at {@link SNAPSHOT_PATH} and 404s
|
||||
* everything else. Port 0, so parallel suites cannot collide.
|
||||
*/
|
||||
export async function startFliptFixtureServer(snapshot: unknown): Promise<FliptFixtureServer> {
|
||||
const received: FliptFixtureServer['received'] = [];
|
||||
const server: Server = createServer((req, res) => {
|
||||
received.push({
|
||||
url: req.url ?? '',
|
||||
environment: req.headers['x-flipt-environment'] as string | undefined,
|
||||
auth: req.headers.authorization as string | undefined,
|
||||
});
|
||||
if (!req.url?.startsWith(SNAPSHOT_PATH)) {
|
||||
res.writeHead(404).end('{}');
|
||||
return;
|
||||
}
|
||||
res.writeHead(200, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify(snapshot));
|
||||
});
|
||||
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
|
||||
const { port } = server.address() as AddressInfo;
|
||||
return {
|
||||
url: `http://127.0.0.1:${port}`,
|
||||
received,
|
||||
close: () => new Promise<void>((resolve) => server.close(() => resolve())),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The body of each suite's `vi.mock('~/server/flipt/client', …)` factory.
|
||||
*
|
||||
* Substitutes ONLY the app's env plumbing (`~/env/server` is not loadable in a unit
|
||||
* run): the returned `isFlipt` is a REAL `createFliptClient` instance pointed at the
|
||||
* fixture server. Everything a defect could live in — the client factory, its cache,
|
||||
* the wasm engine, the segment matcher — stays production code.
|
||||
*
|
||||
* The URL is read from an env var rather than passed, because `vi.mock` factories are
|
||||
* hoisted above every other statement in the file; the suite writes the var in
|
||||
* `beforeAll`, which still runs before the first `await import(…)`.
|
||||
*/
|
||||
export async function buildRealFliptClientMock(urlEnvVar: string) {
|
||||
const { createFliptClient } = await import('@civitai/flipt');
|
||||
const flipt = createFliptClient({
|
||||
url: process.env[urlEnvVar] as string,
|
||||
clientToken: 'test-token',
|
||||
environment: 'civitai-app',
|
||||
log: () => undefined,
|
||||
onInitError: (e) => {
|
||||
throw e;
|
||||
},
|
||||
});
|
||||
return {
|
||||
isFlipt: flipt.isEnabled,
|
||||
isFliptSync: flipt.isEnabledSync,
|
||||
getFliptVariant: flipt.getVariant,
|
||||
getFliptBoolean: flipt.getBoolean,
|
||||
ensureFliptInitialized: flipt.ensureInitialized,
|
||||
};
|
||||
}
|
||||
|
||||
type SnapshotFlag = { key: string; enabled: boolean; [k: string]: unknown };
|
||||
type Snapshot = { namespace: unknown; flags: SnapshotFlag[]; digest?: unknown };
|
||||
|
||||
/**
|
||||
* Derive a snapshot by copying ONE flag's shape out of `source` under new keys with
|
||||
* chosen base `enabled` values.
|
||||
*
|
||||
* 🔴 DERIVED, NEVER HAND-WRITTEN. The base-true case needs the production flag SHAPE
|
||||
* (base + a `SEGMENT_ROLLOUT_TYPE` whose `OR_SEGMENT_OPERATOR` combines the segments)
|
||||
* with only `enabled` changed. A second checked-in fixture would be a copy that
|
||||
* cannot track the original: the source fixture's own docblock says re-capturing it
|
||||
* means re-anonymising it, so a hand-edited twin silently keeps the old segment shape
|
||||
* while still claiming production fidelity — it already lagged a segment by the time
|
||||
* this was written. Copying at runtime makes a re-capture propagate to both suites.
|
||||
*/
|
||||
export function deriveSnapshotFromFlagShape(
|
||||
source: Snapshot,
|
||||
templateKey: string,
|
||||
flags: { key: string; enabled: boolean }[]
|
||||
): Snapshot {
|
||||
const template = source.flags.find((f) => f.key === templateKey);
|
||||
if (!template) {
|
||||
throw new Error(
|
||||
`deriveSnapshotFromFlagShape: no flag '${templateKey}' in the source snapshot ` +
|
||||
`(has: ${source.flags.map((f) => f.key).join(', ')})`
|
||||
);
|
||||
}
|
||||
const rollouts = (template as { rollouts?: { type?: unknown }[] }).rollouts;
|
||||
// 🔴 ASSERT THE ROLLOUT *TYPE*, not merely that some rollout exists. The sentence
|
||||
// this guard enforces is "models a SEGMENT-rolled-out flag", and three weaker tests
|
||||
// all pass while that sentence is false:
|
||||
// - no `rollouts` key at all → a base-`true` flag with no rollouts is an honest
|
||||
// global on-switch: a different shape and a different claim;
|
||||
// - an EMPTY array → same, while passing `Array.isArray`;
|
||||
// - a THRESHOLD (percentage) rollout → the shape `APP_LISTINGS_PUBLIC_EXTERNAL_FLAG`
|
||||
// names as live and hazardous. If the sibling snapshot is ever re-captured with
|
||||
// one, a length check still passes and the base-true suite quietly stops
|
||||
// measuring the segment case — and a 100% threshold would sail past the
|
||||
// `base-false-control` backstop too, because that control would also be `true`.
|
||||
// Every `true` in the consuming suite is attributable to the base value only if the
|
||||
// template really carries a segment rollout, so that is what gets checked.
|
||||
// 🔴 BOTH ARMS, because the threshold case above is NOT removed by requiring a segment
|
||||
// rollout to exist: a re-capture can carry a THRESHOLD rollout ALONGSIDE the segment one,
|
||||
// which `.some()` admits while `base-false-control` then evaluates `true` for everyone —
|
||||
// the precise failure this guard's own comment names. Checking only `.some()` made the
|
||||
// description wider than the implementation.
|
||||
const hasSegment =
|
||||
Array.isArray(rollouts) && rollouts.some((r) => r?.type === 'SEGMENT_ROLLOUT_TYPE');
|
||||
const hasThreshold =
|
||||
Array.isArray(rollouts) && rollouts.some((r) => r?.type === 'THRESHOLD_ROLLOUT_TYPE');
|
||||
if (!hasSegment || hasThreshold) {
|
||||
// The two arms get DISTINCT text so a mutation of either dies for its own reason
|
||||
// rather than to the other's error.
|
||||
// 🔴 `hasSegment &&` is load-bearing and was missing in the first cut of this fix: a
|
||||
// THRESHOLD-only template has no segment rollout either, so keying the message on
|
||||
// `hasThreshold` alone made it announce a rollout "alongside its segment rollout"
|
||||
// that is not there. The existing FIXTURE-GUARD CONTROL arm caught it.
|
||||
throw new Error(
|
||||
hasSegment && hasThreshold
|
||||
? `deriveSnapshotFromFlagShape: template flag '${templateKey}' carries a ` +
|
||||
`THRESHOLD_ROLLOUT_TYPE rollout alongside its segment rollout — a percentage ` +
|
||||
`ramp makes base-false-control evaluate true for every subject, so the ` +
|
||||
`consuming suite would stop attributing its results to the base value`
|
||||
: `deriveSnapshotFromFlagShape: template flag '${templateKey}' carries no ` +
|
||||
`SEGMENT_ROLLOUT_TYPE rollout — the derived snapshot would not model a ` +
|
||||
`segment-rolled-out flag at all`
|
||||
);
|
||||
}
|
||||
return {
|
||||
namespace: source.namespace,
|
||||
flags: flags.map(({ key, enabled }) => ({
|
||||
...(structuredClone(template) as SnapshotFlag),
|
||||
key,
|
||||
enabled,
|
||||
})),
|
||||
digest: 'derived-from-' + templateKey,
|
||||
};
|
||||
}
|
||||
@@ -13,6 +13,53 @@ import {
|
||||
|
||||
const APP_BLOCKS_FLAG = 'app-blocks-enabled';
|
||||
|
||||
/**
|
||||
* 🔴 GLOBAL-EVAL SEMANTICS — read this before writing "fail-closed" anywhere in
|
||||
* this file. Several docblocks below used to say, in one wording or another:
|
||||
*
|
||||
* "no user → a global eval that can never match a segment → fail-closed"
|
||||
*
|
||||
* The premise is TRUE and the conclusion DOES NOT FOLLOW FROM IT. A no-user call
|
||||
* reaches Flipt as entityId `'global'` with an empty context, and every identity
|
||||
* / tier / cohort segment we have is a `STRING_COMPARISON_TYPE` constraint that
|
||||
* reads the CONTEXT, so none of them can match — that much is right. But when no
|
||||
* rollout matches, Flipt answers with the flag's own base `enabled` value. The
|
||||
* denial therefore comes from the BASE BEING FALSE, not from the segment miss.
|
||||
*
|
||||
* MEASURED against the real `@flipt-io/flipt-client-js` wasm engine over a real
|
||||
* evaluation snapshot (`app-blocks-flag.base-enabled-flip.test.ts`):
|
||||
*
|
||||
* base `enabled: true` + a non-matching SEGMENT_ROLLOUT, no entityId/context → **true**
|
||||
* base `enabled: false` + the same rollout, no entityId/context → **false**
|
||||
* unknown flag key, no entityId/context → **false**
|
||||
*
|
||||
* So the two claims that ARE unconditional, and the only ones worth calling
|
||||
* fail-safe without a qualifier, are:
|
||||
* - an ABSENT flag evaluates `false` (the eval throws; `isEnabled` catches → `false`), and
|
||||
* - an UNREACHABLE Flipt evaluates `false` (`isEnabled` returns `false` on a null client).
|
||||
* A "the segment can't match, so it's closed" claim is conditional on the base
|
||||
* value and must say so. (`FLIPT_LOCAL_OVERRIDES` is a third route to `true` with
|
||||
* no user, but it is hard-disabled when `NODE_ENV === 'production'`.)
|
||||
*
|
||||
* Practical consequence for every no-user branch in this file: it is a request to
|
||||
* read the flag's BASE, nothing more. Where that is what the caller means (the
|
||||
* machine/pipeline/runtime gates, the deliberate anonymous-public widening on
|
||||
* `app-listings-public-external`) the branch is correct and load-bearing. Where
|
||||
* the caller means "deny — there is no subject", the branch must say `false`
|
||||
* itself; `isAppBlocksAuthorEnabled` is the one that does.
|
||||
*
|
||||
* ⚠️ SECOND, UNRELATED READING TRAP IN THIS FILE: most docblocks below carry a
|
||||
* sentence of the form "the flag does NOT exist in Flipt at merge time / yet".
|
||||
* Each was an AS-MERGED note written by the PR that added that flag, so each is a
|
||||
* claim about the day it was written — and flags get created and widened after
|
||||
* merge, which is the whole point of shipping dark. Those sentences are history,
|
||||
* not live state, and the "so the as-merged posture is dark" conclusions they
|
||||
* support expire with them. Never plan on one, and do not replace one with a
|
||||
* fresher enumeration here — this file cannot hold live flag state without
|
||||
* becoming the same trap. Read the definitions from `civitai/flipt-state`
|
||||
* (`civitai-app/default/features.yaml`); the live answer is Flipt itself.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Dedicated App Store VISIBILITY flag (W13 — PR-W1a / D8).
|
||||
*
|
||||
@@ -187,12 +234,36 @@ export const APP_BLOCKS_RUNTIME_FLAG = 'app-blocks-runtime-enabled';
|
||||
* For all machine gates, do NOT fabricate user context (the no-arg overload
|
||||
* below, and the pipeline helper, preserve the global-eval behaviour).
|
||||
*
|
||||
* 🔴 The no-user branch here is a request for `app-blocks-enabled`'s BASE
|
||||
* value, not a guaranteed deny. It is KEPT — unlike `isAppBlocksAuthorEnabled`,
|
||||
* whose `user` parameter is REQUIRED — and the reason is SEMANTIC, not a head
|
||||
* count of callers. This flag is a KILL-SWITCH: it answers "is the feature on
|
||||
* at all", a question a subject-less machine path can legitimately ask, and the
|
||||
* flag's base value IS that answer. `app-blocks-author` is a CAPABILITY: it
|
||||
* answers "may THIS subject author", which is unanswerable without a subject,
|
||||
* so there the absence of one is a type error rather than a `false`.
|
||||
*
|
||||
* (The only no-arg call site is `pages/api/v1/developer/block-manifests.ts` —
|
||||
* the JOB_TOKEN manifest registrar, which is DORMANT: nothing in this repo
|
||||
* outside tests and docs invokes that endpoint. Do not rest the asymmetry on
|
||||
* that caller existing; rest it on the kill-switch/capability distinction
|
||||
* above, which survives the endpoint being deleted.)
|
||||
*
|
||||
* The consequence to hold on to: at a base-`enabled: true` GA flip every
|
||||
* no-user caller of THIS helper starts passing. That is the intended reading
|
||||
* for a kill-switch, and it is why the identity-shaped callers must not route a
|
||||
* missing subject through it — see GLOBAL-EVAL SEMANTICS at the top of this
|
||||
* file, and `blocks.router.ts::assertAppBlocksEnabledForTokenUser`, which
|
||||
* refuses an unhydratable subject before it gets here.
|
||||
*
|
||||
* The FLAG_OVERRIDE/local-overrides env exists for unit tests + local dev that
|
||||
* need to flip the flag without standing up Flipt.
|
||||
*/
|
||||
export async function isAppBlocksEnabled(opts?: { user?: SessionUser }): Promise<boolean> {
|
||||
// No user supplied → preserve the original global eval for the machine /
|
||||
// anonymous gates (webhooks, JWKS). Their callers are unchanged.
|
||||
// anonymous gates (webhooks, JWKS). Their callers are unchanged. This returns
|
||||
// the flag's BASE value, so it opens at a base-`enabled` flip — deliberate for
|
||||
// a kill-switch, NOT a deny. See GLOBAL-EVAL SEMANTICS at the top of this file.
|
||||
if (!opts?.user) {
|
||||
return isFlipt(APP_BLOCKS_FLAG);
|
||||
}
|
||||
@@ -222,12 +293,15 @@ export async function isAppBlocksEnabled(opts?: { user?: SessionUser }): Promise
|
||||
* difference: if `app-listings` resolves `false`, this FALLS BACK to
|
||||
* `isAppBlocksEnabled(opts)`. That fallback is the whole point of the dark
|
||||
* decoupling:
|
||||
* - The `app-listings` flag does NOT exist in Flipt at merge time (created
|
||||
* AFTER, as a companion `flipt-state` PR). A bare eval of an absent flag
|
||||
* resolves `false` for EVERYONE — which would REGRESS the currently-visible
|
||||
* cohort (mods + the `app-dev-testers` segment of `app-blocks-enabled`) the
|
||||
* instant this merges. The OR-fallback to `app-blocks-enabled` preserves
|
||||
* their store access verbatim through the transition window.
|
||||
* - The `app-listings` flag did NOT exist in Flipt when this merged (it was
|
||||
* created AFTER, as a companion `flipt-state` PR). A bare eval of an absent
|
||||
* flag resolves `false` for EVERYONE — which would have REGRESSED the
|
||||
* then-visible cohort (mods + the `app-dev-testers` segment of
|
||||
* `app-blocks-enabled`) the instant this merged. The OR-fallback to
|
||||
* `app-blocks-enabled` preserved their store access verbatim through that
|
||||
* transition window. (Past tense on purpose: this is an as-merged note, not
|
||||
* live state — see the reading trap at the top of this file. The paragraph
|
||||
* below says what closes this TODAY.)
|
||||
* - Because `app-blocks-enabled` already grants the mods + app-dev-testers
|
||||
* cohort today, `isAppListingsEnabled` grants EXACTLY that same set until the
|
||||
* `app-listings` flag is created and later widened — so the as-merged change
|
||||
@@ -237,19 +311,24 @@ export async function isAppBlocksEnabled(opts?: { user?: SessionUser }): Promise
|
||||
* the `app-blocks-enabled` cohort (i.e. once `app-listings` is the sole, wider
|
||||
* source of truth); until then the fallback is what keeps existing viewers in.
|
||||
*
|
||||
* No user → preserve a global eval of `app-listings` that can never match a
|
||||
* segment, then fall through to the no-arg `isAppBlocksEnabled()` global eval —
|
||||
* fail-closed, identical to today's no-arg store-read behaviour.
|
||||
* No user → a global eval of `app-listings`, then a fall-through to the no-arg
|
||||
* `isAppBlocksEnabled()` global eval. That is byte-identical to the pre-existing
|
||||
* no-arg store-read behaviour, which is why it is kept. It is NOT unconditionally
|
||||
* fail-closed: both evals return their flag's BASE value, so a base-`enabled`
|
||||
* flip of either key opens the anonymous store read. Both are base-`false` today
|
||||
* with segment rollouts, which is the whole of what makes this dark. See
|
||||
* GLOBAL-EVAL SEMANTICS at the top of this file.
|
||||
*/
|
||||
export async function isAppListingsEnabled(opts?: { user?: SessionUser }): Promise<boolean> {
|
||||
const user = opts?.user;
|
||||
// Per-user eval of the dedicated visibility flag — same entityId + context
|
||||
// shape as isAppBlocksEnabled, so the `app-listings` segment resolves
|
||||
// identically to the client/hasFeature gate. No user → global eval (never
|
||||
// matches a segment).
|
||||
// identically to the client/hasFeature gate.
|
||||
const listingsOn = user
|
||||
? await isFlipt(APP_LISTINGS_FLAG, String(user.id), buildFliptContext(user))
|
||||
: await isFlipt(APP_LISTINGS_FLAG);
|
||||
: // No user → global eval, i.e. the flag's BASE value (not a guaranteed
|
||||
// `false` — see GLOBAL-EVAL SEMANTICS at the top of this file).
|
||||
await isFlipt(APP_LISTINGS_FLAG);
|
||||
if (listingsOn) return true;
|
||||
// OR-fallback: the `app-listings` flag doesn't exist yet (dark window) / hasn't
|
||||
// been widened, so defer to `app-blocks-enabled` to keep the existing
|
||||
@@ -284,19 +363,51 @@ export async function isAppListingsEnabled(opts?: { user?: SessionUser }): Promi
|
||||
* when Flipt returns null (flag absent / Flipt down). So SSR/`ctx.features`
|
||||
* gates and this helper agree in the fail-closed direction: mods only.
|
||||
*
|
||||
* Fail-CLOSED: a non-mod with no `app-blocks-author` grant (flag absent, Flipt
|
||||
* down, or segment miss) → `isFlipt` false → denied. Only mods (floor) and the
|
||||
* flag-granted cohort pass. A vanished/undefined user → no floor + global eval
|
||||
* (can never match a segment) → denied.
|
||||
* ## Fail-closed — and what actually makes it so
|
||||
*
|
||||
* A non-mod with no `app-blocks-author` grant is denied, and that holds under
|
||||
* every flag state: an ABSENT flag and an unreachable Flipt both make `isFlipt`
|
||||
* return `false` unconditionally — see `createFliptClient().isEnabled`, which
|
||||
* returns `false` when the client is null and when the evaluation throws. That
|
||||
* half is independent of how the flag is configured.
|
||||
*
|
||||
* 🔴 THERE IS NO NO-USER BRANCH, AND THE COMPILER IS WHAT GUARANTEES THAT.
|
||||
* `user` is REQUIRED and non-nullable. That is the entire guard: a capability has
|
||||
* nothing to authorize without a subject, so a caller holding a nullable one
|
||||
* cannot reach this function until it has said, in code, what it wants to happen.
|
||||
*
|
||||
* This docblock used to say a vanished/undefined user was denied because of "no
|
||||
* floor + global eval (can never match a segment) → denied". The premise is true
|
||||
* — a no-user eval carries entityId `'global'` and an empty context, so no
|
||||
* `STRING_COMPARISON_TYPE` segment (which is every identity / tier / cohort
|
||||
* segment we have) can match it. The conclusion did NOT follow from it: it
|
||||
* followed from the flag's BASE VALUE being `false`. When no rollout matches,
|
||||
* Flipt answers with the flag's own base `enabled`, so under a base-`enabled:
|
||||
* true` flip that branch resolved TRUE and admitted a caller with no resolvable
|
||||
* subject through an AUTHZ gate. See GLOBAL-EVAL SEMANTICS at the top of this
|
||||
* file for the measurement and for the production-Flipt precedent.
|
||||
*
|
||||
* Why a REQUIRED parameter rather than a `if (!user) return false` branch: the
|
||||
* branch answers for the caller, silently, and every one of them wants to answer
|
||||
* for itself (refuse a vanished token subject / refuse an unauthenticated
|
||||
* request). A required parameter turns each of those into a compile error until
|
||||
* the intent is written down, and it cannot be walked by rewording — unlike the
|
||||
* branch, which reads as handled at every call site without any of them having
|
||||
* decided anything. Making it required errored at exactly 2 of the 10 call sites,
|
||||
* both bare `middleware(...)` whose `ctx.user` type is not narrowed by the
|
||||
* `protectedProcedure` they are attached to; both now refuse explicitly.
|
||||
*
|
||||
* 🔴 What this does NOT stop: a deliberate `user!` or `as SessionUser` cast. At
|
||||
* runtime such a call throws inside `buildFliptContext` / `String(user.id)`
|
||||
* rather than returning `true`, so it still cannot open the gate — but it is a
|
||||
* crash, not a refusal, and review is the only thing that catches the cast.
|
||||
*/
|
||||
export async function isAppBlocksAuthorEnabled(opts?: { user?: SessionUser }): Promise<boolean> {
|
||||
const user = opts?.user;
|
||||
export async function isAppBlocksAuthorEnabled(opts: { user: SessionUser }): Promise<boolean> {
|
||||
const user = opts.user;
|
||||
// Moderator floor — the `availability: ['mod']` static fallback. Keeps mods'
|
||||
// existing author access intact while the Flipt flag is absent (dark window)
|
||||
// and regardless of how the flag's segments are later configured.
|
||||
if (user?.isModerator) return true;
|
||||
// No user → preserve a global eval that can never match a segment (fail-closed).
|
||||
if (!user) return isFlipt(APP_BLOCKS_AUTHOR_FLAG);
|
||||
if (user.isModerator) return true;
|
||||
// Per-user eval — same entityId + context shape as isAppBlocksEnabled, so the
|
||||
// author cohort segment resolves identically to the client/hasFeature gate.
|
||||
return isFlipt(APP_BLOCKS_AUTHOR_FLAG, String(user.id), buildFliptContext(user));
|
||||
@@ -345,9 +456,13 @@ export async function isAppBlocksPipelineEnabled(): Promise<boolean> {
|
||||
*
|
||||
* OPERATOR NOTE: create `app-blocks-runtime-enabled` in Flipt as a PLAIN GLOBAL
|
||||
* BOOLEAN (base `enabled`, NO segment) — this helper evals globally
|
||||
* (`entityId='global'`, empty context), so a segment-targeted flag would never
|
||||
* match and resolve `false`, silently leaving runtime DARK (blocks mysteriously
|
||||
* fail to verify). Fail-safe direction, but a confusing misconfig.
|
||||
* (`entityId='global'`, empty context), so no segment can ever match it and the
|
||||
* answer is always the flag's BASE value. A base-`false` flag carrying a segment
|
||||
* rollout therefore resolves `false` for everyone, silently leaving runtime DARK
|
||||
* (blocks mysteriously fail to verify). ⚠️ The reverse misconfig is NOT
|
||||
* fail-safe: base `true` PLUS a segment resolves `true` globally — the segment
|
||||
* looks like a restriction and restricts nothing. Set the base, don't decorate
|
||||
* it. See GLOBAL-EVAL SEMANTICS at the top of this file.
|
||||
*
|
||||
* Fail-safe: if `app-blocks-runtime-enabled` does not exist (it is created in
|
||||
* Flipt only AFTER this merges) or Flipt is unreachable, `isFlipt` returns
|
||||
@@ -397,14 +512,18 @@ export const APP_BLOCKS_DEV_TUNNEL_FLAG = 'app-blocks-dev-tunnel';
|
||||
* Segment-gated gate for the APP DEV TUNNEL. Evaluated WITH the caller's context
|
||||
* (entityId = user id, context carries server-side `isModerator`) so the
|
||||
* `moderators` / `app-dev-testers` segments can match — identical eval shape to
|
||||
* `isAppBlocksReviewSandboxEnabled`. No user → preserves a global eval that can
|
||||
* never match a segment (fail-closed). See APP_BLOCKS_DEV_TUNNEL_FLAG.
|
||||
* `isAppBlocksReviewSandboxEnabled`. No user → a global eval, which returns the
|
||||
* flag's BASE value; that is `false` today (base OFF + segment rollout) and it is
|
||||
* the base, not the segment miss, that closes it — see GLOBAL-EVAL SEMANTICS at
|
||||
* the top of this file. An absent flag, and an unreachable Flipt, each evaluate
|
||||
* `false` unconditionally — that half IS fail-closed, whatever the base value.
|
||||
* See APP_BLOCKS_DEV_TUNNEL_FLAG.
|
||||
*
|
||||
* NOTE: unlike `isAppBlocksAuthorEnabled`, there is NO moderator static floor —
|
||||
* the flag is created as the rollout, so an absent flag resolves `false` for
|
||||
* EVERYONE (mods included). That is intentional and load-bearing: the dev tunnel
|
||||
* is a brand-new surface (no existing mod access to preserve), so fail-closed for
|
||||
* all until the flag exists is the safe posture.
|
||||
* is a brand-new surface (no existing mod access to preserve), so denying
|
||||
* everyone whenever the flag cannot be evaluated is the safe posture.
|
||||
*/
|
||||
export async function isAppBlocksDevTunnelEnabled(opts?: { user?: SessionUser }): Promise<boolean> {
|
||||
if (!opts?.user) return isFlipt(APP_BLOCKS_DEV_TUNNEL_FLAG);
|
||||
@@ -472,7 +591,11 @@ export async function isAppBlocksDevTunnelUnsubmittedSpendEnabled(opts?: {
|
||||
*
|
||||
* Evaluated globally (entityId='global', empty context), mirroring
|
||||
* `isAppBlocksPipelineEnabled` exactly — so it must be a PLAIN base-`enabled`
|
||||
* boolean in Flipt (NOT segmented), or it would never resolve true.
|
||||
* boolean in Flipt. A segment can never match a global eval, so a segment is not
|
||||
* a way to turn this on, and — the half that matters more — it is not a way to
|
||||
* keep it off either: the global answer is the BASE value, so base `true` plus a
|
||||
* segment arms the reader for everyone. See GLOBAL-EVAL SEMANTICS at the top of
|
||||
* this file.
|
||||
*
|
||||
* Fail-safe: the flag does NOT exist in Flipt yet (it is created only AFTER
|
||||
* this merges, and only when leadership has signed off a rate), or Flipt is
|
||||
@@ -528,8 +651,12 @@ export const APP_BLOCKS_REVIEW_SANDBOX_FLAG = 'app-blocks-review-sandbox-enabled
|
||||
* Mod-segmented gate for the MOD REVIEW SANDBOX (#2831). Evaluated WITH the
|
||||
* moderator's context (entityId = user id, context carries server-side
|
||||
* `isModerator`) so the `moderators` segment can match — identical eval shape to
|
||||
* `isAppBlocksEnabled({ user })`. No user → preserves a global eval that can
|
||||
* never match the segment (fail-closed). See APP_BLOCKS_REVIEW_SANDBOX_FLAG.
|
||||
* `isAppBlocksEnabled({ user })`. No user → a global eval, which returns the
|
||||
* flag's BASE value — `false` today because the flag is base OFF with a segment
|
||||
* rollout, not because the segment cannot match. See GLOBAL-EVAL SEMANTICS at the
|
||||
* top of this file, and APP_BLOCKS_REVIEW_SANDBOX_FLAG (whose "a plain-boolean
|
||||
* global flag would also work" note is exactly the shape that would open this
|
||||
* branch).
|
||||
*/
|
||||
export async function isAppBlocksReviewSandboxEnabled(opts?: {
|
||||
user?: SessionUser;
|
||||
@@ -569,9 +696,11 @@ export const APP_BLOCKS_AGENTIC_REVIEW_FLAG = 'app-blocks-agentic-review';
|
||||
* Mod-segmented gate for the AGENTIC MOD CODE-REVIEW (App Blocks P1). Evaluated
|
||||
* WITH the moderator's context (entityId = user id, context carries server-side
|
||||
* `isModerator`) so the `moderators` segment can match — identical eval shape to
|
||||
* `isAppBlocksReviewSandboxEnabled({ user })`. No user → preserves a global eval
|
||||
* that can never match the segment (fail-closed), and an absent flag also
|
||||
* evaluates false (fail-closed). See APP_BLOCKS_AGENTIC_REVIEW_FLAG.
|
||||
* `isAppBlocksReviewSandboxEnabled({ user })`. An absent flag, and an unreachable
|
||||
* Flipt, each evaluate `false` unconditionally — that half IS fail-closed. No user
|
||||
* → a global eval, which returns the flag's BASE value; base OFF plus a segment
|
||||
* rollout is what keeps that closed, not the segment miss. See GLOBAL-EVAL
|
||||
* SEMANTICS at the top of this file, and APP_BLOCKS_AGENTIC_REVIEW_FLAG.
|
||||
*/
|
||||
export async function isAppBlocksAgenticReviewEnabled(opts?: {
|
||||
user?: SessionUser;
|
||||
@@ -594,15 +723,21 @@ export async function isAppBlocksAgenticReviewEnabled(opts?: {
|
||||
* Evaluated WITH the caller's context (entityId = user id, context carries
|
||||
* server-side `isModerator`) so the `moderators` / community segments can match.
|
||||
* On the block-token path the "caller" is the HYDRATED TOKEN SUBJECT
|
||||
* (`getSessionUserById`), not a session — anon reads pass no user → global eval
|
||||
* that can never match a segment → fail-closed (anon shared access is a GA-only
|
||||
* widening, safe to stay dark until a base-`enabled` flip).
|
||||
* (`getSessionUserById`), not a session — anon reads pass no user → global eval,
|
||||
* which returns the flag's BASE value. Closed today because the base is `false`;
|
||||
* a base-`enabled` flip DOES open anon shared reads, and that is the intended
|
||||
* GA widening rather than an accident — the qualifier this docblock already
|
||||
* carried ("safe to stay dark until a base-`enabled` flip") is the accurate half,
|
||||
* so do not read the word fail-closed into the segment miss. See GLOBAL-EVAL
|
||||
* SEMANTICS at the top of this file.
|
||||
*
|
||||
* Create it in Flipt as base `enabled: false` with the `moderators` segment (+
|
||||
* any community-cohort segment) exactly like `app-blocks-dev-tunnel`. The flag
|
||||
* does NOT exist in Flipt at merge time — the companion `flipt-state` entry is a
|
||||
* SEPARATE follow-up PR — so the as-merged posture is fully dark and cannot
|
||||
* regress the gate open.
|
||||
* did NOT exist in Flipt when this merged — the companion `flipt-state` entry was
|
||||
* a SEPARATE follow-up PR — so the as-merged posture was fully dark and could not
|
||||
* regress the gate open. (Past tense on purpose: this is an as-merged note, not
|
||||
* live state — see the reading trap at the top of this file. The paragraph above
|
||||
* says what closes this TODAY.)
|
||||
*/
|
||||
export const APP_BLOCKS_SHARED_STORAGE_FLAG = 'app-blocks-shared-storage';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user