fix(blocks): an unreadable post subject is no longer reported as "posting from apps is not enabled" (#4976)

`authorizeBlockPostRequest` passed an unhydratable session subject straight
into `isAppBlocksPostCreationEnabled({ user: subjectUser ?? undefined })` --
the no-entity arm. A segment-scoped rollout cannot match a no-entity eval, so
it answers false, and a failed identity read was rendered to the viewer as
"posting from apps is not enabled". Two different facts, one message, and only
one of them is about permission.

Refuse an unhydratable subject on its own terms before the flag is consulted,
matching the two sibling guards in this router family. The flag denial keeps
its message unchanged, so the two stay separable.

This does NOT widen who may post: an unreadable subject is still refused, and
a dedicated case pins that it is still refused under a base-enabled flag. It
also does not assert a mechanism for any particular production refusal -- a
subject carrying a stale isModerator and a transient flag-evaluation failure
produce the identical observable and neither is excluded.

- BlockPostRequestAuth.subjectUser is now non-nullable, so widening it back is
  a type error rather than a silent re-conflation.
- New counter civitai_app_block_post_subject_refusals_total{surface}, following
  the existing emitters in app-block-runtime.metrics.ts. A log line would not
  have worked: application-container logs are not collected for this
  deployment, which is why the original refusal was unattributable.
- Watchlisted as post-subject-refusal -- losing the branch to a bundler falls
  back to the no-entity arm, which returns the flag's BASE value.

Regression matrix (5 of 6 new cases): red at 85a3786116, green at HEAD.
The sixth, the flag-denial positive control, passes on both trees and is an
invariant guard, not regression coverage.
This commit is contained in:
Zachary Lowden
2026-09-19 11:33:40 -05:00
committed by GitHub
parent 2df0241a4f
commit a75fb42290
6 changed files with 454 additions and 10 deletions
+17
View File
@@ -154,4 +154,21 @@ export const COMPILED_BRANCH_WATCHLIST = [
},
],
},
{
id: 'post-subject-refusal',
module: 'src/server/routers/blocks.router.ts',
why: "`authorizeBlockPostRequest` refuses a post/preview whose token subject does not hydrate, BEFORE consulting `app-blocks-post-creation`. Lost, it falls back to the shape this branch replaced — the flag evaluated with no entity and no context, which returns the flag's BASE value rather than a deny. Under a base-`enabled: true` GA flip of post creation, a token whose subject no longer resolves would then be allowed to publish a PUBLIC, feed-visible post under that subject's byline. Same hazard as `shared-storage-subject-refusal` and `block-token-subject-refusal`, on the one surface where the consequence is public content rather than a read. NB the TypeScript cannot cover this: `BlockPostRequestAuth.subjectUser` is non-nullable, so deleting the branch in SOURCE is a type error — which is exactly why only an emitted-output gate can see a bundler dropping it.",
control: [
{
code: 'isAppBlocksPostCreationEnabled({ user: subjectUser }',
why: 'the flag call immediately after the refusal — same function, known to survive. Unmapped means the gate is looking at a build that never emitted this function, not at a violation. Stops before the closing paren so argument reflow cannot move it off this line.',
},
],
required: [
{
code: 'if (!subjectUser) {',
why: "the refusal's own CONDITION rather than a payload inside it, per rule 2 of this file's header — a condition cannot be interned from another site. It is unique in this module today (the sibling `assertViewerIsAppDeveloper` refusal spells its binding `user`, not `subjectUser`); if a second `subjectUser` null-check is ever added to this router, re-point this anchor at the message literal instead, which is unique app-wide.",
},
],
},
];
@@ -0,0 +1,114 @@
import client from 'prom-client';
import { beforeEach, describe, expect, it } from 'vitest';
import {
APP_BLOCK_POST_SURFACES,
ensureRegisterAppBlockRuntimeMetrics,
recordBlockPostSubjectRefusal,
type AppBlockPostSurface,
} from '../app-block-runtime.metrics';
/**
* The REAL prom-client side of the post-from-app UNREADABLE-SUBJECT signal.
*
* ⚠️ LABEL THIS FILE HONESTLY: it is NOT regression coverage. The counter it checks
* does not exist on the pre-change tree, so nothing here can be watched failing against
* a build that had the defect — these are guards on a brand-new surface. The regression
* matrix belongs to the router cases in
* `src/server/routers/__tests__/blocks.router.createPostFromApp.test.ts`, which run
* against both trees.
*
* WHY IT EXISTS ANYWAY, in the words its three siblings already use for this class: the
* router-level test proves the preamble CALLS the emitter on the refusal path. It cannot
* prove the emitter produces a SCRAPEABLE SERIES with the right name and the right label.
* A metric-name typo or a label-name mismatch sails straight through that test and yields
* an alert rule that silently never fires.
*
* 🔴 AND THIS EMITTER IS THE WORST CASE FOR THAT, for three compounding reasons:
* - It is `try { … } catch {}` by design (a metrics error must never convert a refusal
* the preamble already settled into a 500), so a registration or label defect
* produces ZERO and never throws. Nothing anywhere would go red.
* - A flat zero is ALSO the healthy steady state — a subject that hydrates never
* reaches the emitter. So "the counter is inert" and "the fleet is fine" are the same
* observation, and no amount of watching the series can separate them.
* - 🔴 There is no second surface to fall back on. Application-container logs are not
* collected for this deployment, so unlike every other refusal in this router there
* is no log line an investigator can read instead. If this series is broken, the
* branch is exactly as unobservable as it was before the fix — which is the state
* that left the 2026-09-19 refusal unattributable to any mechanism.
*
* 🔴 The cardinality bound is the other load-bearing property, same as the siblings. One
* label over a 2-value code-owned union = 2 series, total, forever. prom-client retains
* every distinct label set in the Node heap for the process lifetime across every scraped
* pod, so widening this is a code change that has to get past these tests.
*/
const METRIC = 'civitai_app_block_post_subject_refusals_total';
/** Read one `{surface}` series' current value from the default registry. */
async function readSurface(surface: string): Promise<number> {
const metric = client.register.getSingleMetric(METRIC) as
| { get(): Promise<{ values: Array<{ labels: Record<string, string>; value: number }> }> }
| undefined;
if (!metric) return Number.NaN;
const { values } = await metric.get();
return values.find((v) => v.labels.surface === surface)?.value ?? 0;
}
beforeEach(() => {
// `clear()` (not `resetMetrics()`), matching the sibling suites: a fully empty default
// registry makes each case order-independent even when one of them registers something
// under this name itself.
client.register.clear();
});
describe(METRIC, () => {
it('is registered on the default registry that /api/metrics scrapes', () => {
ensureRegisterAppBlockRuntimeMetrics();
expect(client.register.getSingleMetric(METRIC)).toBeDefined();
});
it.each(APP_BLOCK_POST_SURFACES.map((s) => [s] as [AppBlockPostSurface]))(
'increments the `%s` series',
async (surface) => {
recordBlockPostSubjectRefusal(surface);
expect(await readSurface(surface)).toBe(1);
}
);
it('🔴 keeps the two surfaces SEPARATE — a create refusal never lands on preview', async () => {
// The split is the whole value of the label. A refused `create` is a post the viewer
// intended to make and did not get; a refused `preview` cost them a dialog. Summed
// across the label those are one number with no operational meaning.
recordBlockPostSubjectRefusal('create');
recordBlockPostSubjectRefusal('create');
recordBlockPostSubjectRefusal('preview');
expect(await readSurface('create')).toBe(2);
expect(await readSurface('preview')).toBe(1);
});
it('carries ONLY `surface` — no app, block-instance or user label', async () => {
// Cardinality bound, pinned rather than commented. This fires once per refused
// attempt with nothing caching it; an id-shaped label here is the Node-heap growth
// class the module header calls out.
recordBlockPostSubjectRefusal('create');
const metric = client.register.getSingleMetric(METRIC) as unknown as {
get(): Promise<{ values: Array<{ labels: Record<string, string> }> }>;
};
const { values } = await metric.get();
expect(values.length).toBeGreaterThan(0);
for (const v of values) expect(Object.keys(v.labels).sort()).toEqual(['surface']);
});
it('NEGATIVE CONTROL: it does not throw when the registry is poisoned, and the caller survives', () => {
// The emitter is deliberately fail-soft — the refusal is already decided by the time
// it runs, and a metrics error must not turn a chosen 401 into an uncaught 500. This
// is also why a broken emitter is silent, and therefore why every case above has to
// be a real scrape rather than a spy on the call.
client.register.registerMetric(
new client.Gauge({ name: METRIC, help: 'poisoned — wrong type, same name' })
);
expect(() => recordBlockPostSubjectRefusal('create')).not.toThrow();
});
});
@@ -282,6 +282,21 @@ export function revocationNamespaceLabel(blockInstanceId: unknown): AppBlockRevo
return 'other';
}
/**
* The two post-from-app doors whose shared preamble can refuse an unhydratable
* token subject: `blocks.createPostFromApp` (the write) and
* `blocks.previewPostFromApp` (the read-only dry run).
*
* 🔴 THE SPLIT IS LOAD-BEARING AND NOT COSMETIC. Both procs run the identical
* preamble, so a combined number would leave an operator unable to say whether a
* spike cost anyone a real post. A refused `create` is a post the viewer intended
* to make and did not get; a refused `preview` cost them a dialog. Those warrant
* different urgency, and the label is the only thing that can tell them apart —
* there is no per-request log to fall back on for this deployment.
*/
export const APP_BLOCK_POST_SURFACES = ['preview', 'create'] as const;
export type AppBlockPostSurface = (typeof APP_BLOCK_POST_SURFACES)[number];
export const APP_BLOCK_REST_APPROVAL_VERDICT_REASONS = [
'not_approved',
'not_found',
@@ -465,6 +480,7 @@ type Bundle = {
spendCapRejectionsTotal: Counter<string>;
restApprovalVerdictsTotal: Counter<string>;
revocationRefusalsTotal: Counter<string>;
postSubjectRefusalsTotal: Counter<string>;
stepPriceCheckTotal: Counter<string>;
launchTotalSeconds: Histogram<string>;
launchPhaseSeconds: Histogram<string>;
@@ -828,6 +844,42 @@ export function ensureRegisterAppBlockRuntimeMetrics(reg: Registry = client.regi
['surface', 'namespace']
);
// ── POST-FROM-APP: UNREADABLE SUBJECT ────────────────────────────────────────
// 🔴 THIS BRANCH WAS PREVIOUSLY INDISTINGUISHABLE FROM A FLAG DENIAL, AND THAT IS
// THE DEFECT THIS COUNTER EXISTS TO MAKE READABLE. `authorizeBlockPostRequest`
// used to pass an unhydratable subject on to the post-creation flag as
// `{ user: undefined }` — the no-entity arm — and a segment-scoped rollout
// answers `false` to a no-entity eval, so the viewer was told "posting from apps
// is not enabled" whenever their session could not be read. Two different facts,
// one message; only one of them is about permission.
//
// 🔴 A LOG LINE WOULD NOT HAVE SATISFIED THIS. Application-container stdout is
// not collected into the log store for this deployment, so a `console.error` on
// this branch is unreadable to any later investigator — the exact reason the
// 2026-09-19 refusal could not be attributed to a mechanism at all. A scraped
// counter is the only surface that exists here today.
//
// 🔴 ONE LABEL, `surface`, over a 2-value code-owned union → 2 series, TOTAL. No
// `app_block_id` and no user id: this fires once per refused post/preview attempt
// with nothing caching it, and prom-client retains every distinct label set in
// the Node heap for the process lifetime across every scraped pod. Same
// alert-on-the-metric / attribute-from-the-log split as the two counters above —
// except that here the log half does not exist yet, so read this series as a RATE
// signal only and do not expect to identify WHICH viewer was refused from it.
//
// 🔴 WHAT A ZERO DOES AND DOES NOT MEAN. Zero is the healthy steady state — a
// subject that hydrates never reaches the emitter — so "nothing has gone wrong"
// and "the emitter is inert" are the same observation on this series alone. That
// is why the registration itself is pinned by a real-registry scrape in
// `app-block-post-subject-refusals.metrics.test.ts` rather than left to the
// caller-level test.
const postSubjectRefusalsTotal = getOrCreateCounter(
reg,
'civitai_app_block_post_subject_refusals_total',
"Post-from-app requests refused because the token subject did not hydrate to a SessionUser, by surface. surface: create = blocks.createPostFromApp, preview = blocks.previewPostFromApp. This is NOT a flag denial and must never be read as one — a flag denial does not increment this series at all, and the two refusals carry different messages on purpose. A non-zero rate means viewers who may well be entitled to post were turned away by an identity read that failed, so alert on the RATE, not on a single event. Carries no app or user label (cardinality); per-viewer attribution is not available on this deployment because application-container logs are not collected, so this counter is the whole signal. Zero is also the healthy steady state, so a flat zero cannot by itself distinguish 'nothing failed' from 'the emitter is inert' — the registration is pinned by a real-registry test instead",
['surface']
);
// ── `kind: 'step'` prepaidFixed PRICE CHECK ──────────────────────────────────
// 🔴 Instruments whether the registry's DECLARED price still matches what the
// orchestrator actually bills for a `prepaidFixed` step type. A declared price
@@ -995,6 +1047,7 @@ export function ensureRegisterAppBlockRuntimeMetrics(reg: Registry = client.regi
spendCapRejectionsTotal,
restApprovalVerdictsTotal,
revocationRefusalsTotal,
postSubjectRefusalsTotal,
stepPriceCheckTotal,
launchTotalSeconds,
launchPhaseSeconds,
@@ -1267,6 +1320,34 @@ export function recordBlockRestApprovalVerdict(reason: AppBlockRestApprovalVerdi
}
}
/**
* Fail-soft emit of one post-from-app refusal caused by a token subject that did not
* hydrate. Called from the shared post preamble in `blocks.router.ts`.
*
* 🔴 TOTAL, like every emitter here, and for the usual reason: the refusal is already
* decided by the time this runs, so a metrics error must not convert a chosen 401 into
* an uncaught 500.
*
* 🔴 THIS IS THE ONLY OBSERVABILITY THIS BRANCH HAS. Application-container logs are not
* collected for this deployment, so the `console.error` shape used elsewhere in the repo
* would be invisible to a later investigator. Deleting this call does not fail a type
* check and does not fail any test that only asserts the thrown error — it silently
* returns the branch to being unobservable, which is the state that made the 2026-09-19
* refusal unattributable. `app-block-post-subject-refusals.metrics.test.ts` is what
* stops that.
*
* COST: one in-heap counter increment, only on the refusal path. A fleet whose subjects
* all hydrate emits zero.
*/
export function recordBlockPostSubjectRefusal(surface: AppBlockPostSurface): void {
try {
const { postSubjectRefusalsTotal } = ensureRegisterAppBlockRuntimeMetrics();
postSubjectRefusalsTotal.inc({ surface });
} catch {
/* instrument-only — never let a metrics error change the refusal the gate chose */
}
}
/**
* Fail-soft emit of the settled GPU-runtime cost (billed `actual` Buzz) for one
* customComfy gen. Called from the settle service at terminal. A metrics error
@@ -1,3 +1,4 @@
import client from 'prom-client';
import { beforeEach, describe, expect, it, vi } from 'vitest';
/**
@@ -295,8 +296,165 @@ describe('the shared preamble — createPostFromApp', () => {
expect(mockPreviewBlockPost).not.toHaveBeenCalled();
});
});
/**
* 🔴 THE REGRESSION THIS BLOCK EXISTS FOR: an UNREADABLE SUBJECT AND A FLAG DENIAL
* USED TO BE THE SAME REFUSAL.
*
* The preamble read `isAppBlocksPostCreationEnabled({ user: subjectUser ?? undefined })`,
* so a `null` from the session client fell into the flag's no-entity arm — entityId
* 'global', empty context. A SEGMENT-scoped rollout (the live shape) cannot match a
* no-entity eval, so it answers `false`, and the viewer was told the capability was
* switched off when the real state was "we could not read your session". Those are
* different facts and only one of them is about permission.
*
* 🔴 THE FLAG STUB BELOW IS THE LOAD-BEARING PART OF THE FIXTURE, and the default
* stub in `beforeEach` (an unconditional `true`) cannot express this defect at all —
* under it a null subject sails PAST the gate instead of being mis-refused, so a case
* written on the default would be red at both trees for the wrong reason. These cases
* therefore re-stub the flag as the real no-entity semantics: `true` with a user,
* `false` without one. That is what makes the pre-change tree produce the WRONG
* message rather than no message.
*
* 🔴 WHY `mockResolvedValueOnce` AND NOT A PLAIN `null`. The runtime kill-switch
* (`assertAppBlocksEnabledForTokenUser`, NOT mocked in this file) hydrates the same
* subject one step earlier and refuses a `null` with its own message, so a subject
* that never resolves never reaches the branch under test. Reaching it needs the
* SECOND read to disagree with the first — the cached-read-then-network-fetch window,
* or a user deleted between the two awaits. Same construction the author gate's own
* case uses in `blocks.router.flag-gate-hydrate.test.ts`.
*
* ⚠️ These cases do NOT claim that this is what produced any particular production
* refusal. A subject that hydrated carrying a stale `isModerator`, and a transient
* flag-evaluation failure, produce the same observable and are not excluded here.
*/
describe('🔴 an UNREADABLE SUBJECT is not a flag denial', () => {
/** The real no-entity semantics of a segment-scoped rollout. */
function segmentScopedFlag() {
mockIsAppBlocksPostCreationEnabled.mockImplementation(
async (opts?: { user?: unknown }) => !!opts?.user
);
}
/** Hydrates for the kill-switch, then vanishes before the post preamble re-reads it. */
function vanishesOnSecondRead() {
mockGetSessionUser.mockReset();
mockGetSessionUser.mockResolvedValueOnce(trustedUser()).mockResolvedValue(null);
}
it('REFUSES with its OWN message — not "posting from apps is not enabled"', async () => {
segmentScopedFlag();
vanishesOnSecondRead();
await expect(caller().createPostFromApp(INPUT)).rejects.toMatchObject({
code: 'UNAUTHORIZED',
message: 'posting subject could not be resolved, please try again',
});
expect(mockWriteBlockPost).not.toHaveBeenCalled();
});
it('does NOT reach the flag at all — no no-entity evaluation is performed', async () => {
// The other half of the criterion, and the one a message assertion cannot make:
// the fix is not "rewrite the copy on the way out", it is "never ask the flag a
// question it can only answer wrongly". A no-entity eval returns the flag's BASE
// value, so under a base-`enabled: true` widening the old shape would have
// PASSED an unresolvable subject through to a public post.
segmentScopedFlag();
vanishesOnSecondRead();
await expect(caller().createPostFromApp(INPUT)).rejects.toThrow();
expect(mockIsAppBlocksPostCreationEnabled).not.toHaveBeenCalled();
});
it('🔴 STILL REFUSES under a base-enabled flag — the split does not widen who may post', async () => {
// Non-goal guard. Separating the two verdicts must not become a fail-open: with
// the flag answering `true` to everything (a GA base flip), an unreadable subject
// is still turned away, and the write service is still never reached.
mockIsAppBlocksPostCreationEnabled.mockResolvedValue(true);
vanishesOnSecondRead();
await expect(caller().createPostFromApp(INPUT)).rejects.toMatchObject({
code: 'UNAUTHORIZED',
// The message is asserted here too, not just the code: on the pre-change
// tree this case also rejects — with the write-trust guard's own error,
// several steps later, after the flag had already said yes. Pinning only
// the code would call that red a pass for the wrong reason the moment the
// codes happened to agree.
message: 'posting subject could not be resolved, please try again',
});
expect(mockWriteBlockPost).not.toHaveBeenCalled();
});
it('splits the PREVIEW surface the same way', async () => {
segmentScopedFlag();
vanishesOnSecondRead();
await expect(caller().previewPostFromApp(INPUT)).rejects.toMatchObject({
code: 'UNAUTHORIZED',
message: 'posting subject could not be resolved, please try again',
});
expect(mockPreviewBlockPost).not.toHaveBeenCalled();
});
it('a FLAG DENIAL keeps its own, unchanged message — the two stay separable', async () => {
// The positive control for the split. Without it, every case above is satisfied
// by a build that simply renamed the single refusal, which would move the
// conflation rather than remove it.
segmentScopedFlag();
mockGetSessionUser.mockResolvedValue(trustedUser());
mockIsAppBlocksPostCreationEnabled.mockResolvedValue(false);
await expect(caller().createPostFromApp(INPUT)).rejects.toMatchObject({
code: 'FORBIDDEN',
message: 'posting from apps is not enabled',
});
});
it('is OBSERVABLE — the refusal increments a scraped counter, not just a log line', async () => {
// 🔴 CRITERION 4, AND IT IS READ OFF THE REAL DEFAULT REGISTRY BY METRIC NAME.
// Application-container stdout is not collected for this deployment, so a
// `console.error` here would be unreadable to any later investigator — the
// emitter is the only surface that exists. Looked up by STRING rather than by
// importing the new symbol, deliberately: that keeps this case runnable against
// the pre-change tree, where it fails on a `undefined` registry lookup instead
// of on a module-resolution error.
//
// 🔴 THE EXISTENCE ASSERTION IS NOT DECORATION — WITHOUT IT THIS CASE IS
// VACUOUS. `readPostSubjectRefusals` returns NaN when the series is not
// registered at all, and `expect(NaN).toBe(NaN)` PASSES (`toBe` is
// `Object.is`). Measured: this case went GREEN against the pre-change tree,
// where the counter does not exist, until the `toBeDefined()` below was
// added. A delta assertion over a lookup that can return NaN cannot tell
// "the counter moved" from "there is no counter".
segmentScopedFlag();
vanishesOnSecondRead();
const before = await readPostSubjectRefusals('create');
await expect(caller().createPostFromApp(INPUT)).rejects.toThrow();
expect(client.register.getSingleMetric(POST_SUBJECT_REFUSALS)).toBeDefined();
const after = await readPostSubjectRefusals('create');
expect(Number.isFinite(after)).toBe(true);
expect(after).toBe((Number.isFinite(before) ? before : 0) + 1);
});
});
});
const POST_SUBJECT_REFUSALS = 'civitai_app_block_post_subject_refusals_total';
/**
* Current value of one `{surface}` series of the post-subject refusal counter, or NaN
* when the series is not registered. 🔴 NaN is NOT a safe sentinel for a delta check —
* see the caller.
*/
async function readPostSubjectRefusals(surface: string): Promise<number> {
const metric = client.register.getSingleMetric(POST_SUBJECT_REFUSALS) as
| { get(): Promise<{ values: Array<{ labels: Record<string, string>; value: number }> }> }
| undefined;
if (!metric) return Number.NaN;
const { values } = await metric.get();
return values.find((v) => v.labels.surface === surface)?.value ?? 0;
}
// ─────────────────────────────────────────────────────────────────────────────
describe('write trust', () => {
it.each([
@@ -132,8 +132,14 @@ const {
const { mockRecordStepPriceCheck } = vi.hoisted(() => ({
mockRecordStepPriceCheck: vi.fn(() => undefined),
}));
// A bare factory, so every symbol the router imports from this module must be listed
// here or it resolves to `undefined` and the router throws the moment it is called.
// `recordBlockPostSubjectRefusal` is only reached on the post preamble's unreadable-
// subject branch, which this suite does not drive — it is listed so that stays true by
// construction rather than by luck.
vi.mock('~/server/metrics/app-block-runtime.metrics', () => ({
recordStepPriceCheck: (...a: unknown[]) => mockRecordStepPriceCheck(...(a as [])),
recordBlockPostSubjectRefusal: () => undefined,
}));
vi.mock('~/server/services/blocks/dev-tunnel.service', () => ({
+78 -10
View File
@@ -216,10 +216,20 @@ import {
attachModeratedStepTextOutputs,
runStepModeration,
} from '~/server/services/blocks/steps/moderation';
// Instrument-only: records EVERY prepaidFixed step price check at submit —
// Instrument-only, both of them, and neither ever throws.
// `recordStepPriceCheck` records EVERY prepaidFixed step price check at submit —
// `exact` / `over` / `absent` — so a flat "no divergence" line can be told apart
// from a detector that never ran. Never throws.
import { recordStepPriceCheck } from '~/server/metrics/app-block-runtime.metrics';
// from a detector that never ran.
// `recordBlockPostSubjectRefusal` is the ONLY observability the post preamble's
// unreadable-subject branch has: application-container logs are not collected for
// this deployment, so a log line there would be unreadable to any later
// investigator. Dropping this call silently returns that branch to being
// undiagnosable — it is not a cosmetic emit.
import {
recordBlockPostSubjectRefusal,
recordStepPriceCheck,
type AppBlockPostSurface,
} from '~/server/metrics/app-block-runtime.metrics';
// Post-paid SETTLE-TO-ACTUAL for customComfy (plan §5.3). `persist*` is awaited in
// submit (after reserving the ceiling); `settle*` is a best-effort call on the
// terminal poll/cancel hook. Static import (both are light) — the heavy
@@ -537,14 +547,20 @@ const confirmedImageCountInput = z.number().int().positive().max(100);
* keeps the unauthorized path off the auth hub.
* 3. non-anon subject.
* 4. the App-Blocks RUNTIME flag, evaluated on the token SUBJECT.
* 5. the DEDICATED post-creation flag, also on the subject. Separate from (4)
* 5. the SUBJECT HYDRATES. Refused on its own terms, with its own message, and
* NOT folded into (6) see the docblock on the refusal itself.
* 6. the DEDICATED post-creation flag, also on the subject. Separate from (4)
* on purpose: a GA widening of the runtime flag must not arm public post
* creation on the same day, and this is the per-capability kill switch.
*/
type BlockPostRequestAuth = {
claims: Awaited<ReturnType<typeof authorizeBlockBridgeToken>>;
userId: number;
subjectUser: SessionUser | null;
// NON-NULLABLE ON PURPOSE. Step (5) above refuses a subject that does not
// hydrate, so every caller downstream gets a real `SessionUser` and does not
// have to re-derive what a `null` here would have meant. Widening this back to
// `SessionUser | null` is how the conflated verdict comes back.
subjectUser: SessionUser;
};
/**
@@ -564,7 +580,10 @@ type BlockPostRequestAuth = {
* phantom call site on whatever function happens to sit above it measured, and
* it named `assertAppBlocksEnabledForTokenUser` as a guard caller.
*/
async function authorizeBlockPostRequest(blockToken: string): Promise<BlockPostRequestAuth> {
async function authorizeBlockPostRequest(
blockToken: string,
surface: AppBlockPostSurface
): Promise<BlockPostRequestAuth> {
const claims = await authorizeBlockBridgeToken(blockToken);
// NOT `ai:write:budgeted`. An app authorised to spend the viewer's Buzz on a
// generation has NOT thereby been authorised to publish under their name — the
@@ -583,9 +602,51 @@ async function authorizeBlockPostRequest(blockToken: string): Promise<BlockPostR
await assertAppBlocksEnabledForTokenUser(userId);
const subjectUser = (await sessionClient.getSessionUserById(userId)) as SessionUser | null;
// 🔴 AN UNREADABLE SUBJECT IS ITS OWN REFUSAL, WITH ITS OWN MESSAGE. This used
// to read `{ user: subjectUser ?? undefined }` and fall straight into the flag
// check below, which meant a `null` here was silently re-reported to the viewer
// as "posting from apps is not enabled". It is not: `isAppBlocksPostCreationEnabled`
// with no user takes its no-entity arm — entityId 'global', empty context — and a
// SEGMENT-scoped rollout (the live shape: `moderators`) cannot match a no-entity
// eval, so it answers false. A failed identity read was therefore rendered as a
// policy decision, for a viewer the policy may well admit.
//
// Two different facts, and only one of them is about permission. Separating them
// does NOT widen who may post — a subject we cannot read is still refused, and
// there is deliberately no retry here (a blind retry on a path that creates a
// PUBLIC post under someone's byline is how a duplicate post happens). What
// changes is only what the viewer and the operator are told.
//
// 🔴 THIS IS ALSO A FAIL-CLOSED GUARD IN THE SAME SENSE AS ITS TWO SIBLINGS
// (`assertAppBlocksEnabledForTokenUser`, `assertViewerIsAppDeveloper`): the
// no-entity arm returns the flag's own BASE value, so under a base-`enabled: true`
// GA flip, falling through with no subject would PASS this gate rather than
// refuse. Losing this branch re-opens that. It is watchlisted as
// `post-subject-refusal` in `scripts/compiled-branch-watchlist.mjs`, so the
// message literal below and the condition are both anchors — rewording either is
// a watchlist edit in the same commit, not a copy change.
//
// 🔴 REACHABILITY IS NARROW BUT REAL, AND IS NOT THE JUSTIFICATION ANYWAY. The
// kill-switch above already hydrated this subject and refuses a `null`, so
// reaching here with one needs the second read to disagree with the first: the
// hub-backed resolver is a cached read that falls through to a network fetch on a
// miss, and a user genuinely deleted between the two awaits produces it too — the
// same narrow window `assertViewerIsAppDeveloper` is tested against. 🔴 DO NOT
// ASSERT THAT THIS IS WHAT HAPPENED IN ANY PARTICULAR PRODUCTION REFUSAL. A
// subject that hydrated but carried a stale `isModerator`, and a transient Flipt
// evaluation failure, both produce the identical observable and neither is
// excluded. The counter below is what will let a future investigator tell them
// apart; before it, nothing could.
if (!subjectUser) {
recordBlockPostSubjectRefusal(surface);
throw new TRPCError({
code: 'UNAUTHORIZED',
message: 'posting subject could not be resolved, please try again',
});
}
// Fail-closed: an ABSENT flag resolves false for everyone, mods included, so
// the capability is fully dark until a deliberate Flipt flip.
if (!(await isAppBlocksPostCreationEnabled({ user: subjectUser ?? undefined }))) {
if (!(await isAppBlocksPostCreationEnabled({ user: subjectUser }))) {
throw new TRPCError({ code: 'FORBIDDEN', message: 'posting from apps is not enabled' });
}
@@ -4515,7 +4576,7 @@ export const blocksRouter = router({
previewPostFromApp: publicProcedure
.input(z.object({ blockToken: z.string().min(1), ...blockPostPayloadShape }))
.mutation(async ({ ctx, input }) => {
const { claims, userId } = await authorizeBlockPostRequest(input.blockToken);
const { claims, userId } = await authorizeBlockPostRequest(input.blockToken, 'preview');
// NOTE: the CATALOG bucket, not the post bucket. The preview writes
// nothing, so charging it against the 3-posts/hour ceiling would let a
// block exhaust its own posting budget by rendering dialogs — and the
@@ -4615,13 +4676,20 @@ export const blocksRouter = router({
})
)
.mutation(async ({ ctx, input }) => {
const { claims, userId, subjectUser } = await authorizeBlockPostRequest(input.blockToken);
const { claims, userId, subjectUser } = await authorizeBlockPostRequest(
input.blockToken,
'create'
);
// WRITE TRUST. Reused from the shared-storage path. "Verified email" is
// satisfied by emailVerified OR a linked OAuth account; only query for the
// link when emailVerified is absent, so a verified-email user pays nothing.
// No `subjectUser &&` guard here any more: the preamble refuses an
// unhydratable subject outright, so this is a real `SessionUser` by
// construction. Re-adding the null check would be dead code that quietly
// claims the opposite.
let hasLinkedOAuth = false;
if (subjectUser && !subjectUser.emailVerified) {
if (!subjectUser.emailVerified) {
hasLinkedOAuth = (await dbRead.account.count({ where: { userId } })) > 0;
}
assertSharedWriteTrust(subjectUser, hasLinkedOAuth);