diff --git a/.claude/agents/civitai-test-review.md b/.claude/agents/civitai-test-review.md index abc44d78dc..8d5f7324ef 100644 --- a/.claude/agents/civitai-test-review.md +++ b/.claude/agents/civitai-test-review.md @@ -178,8 +178,8 @@ Worked examples of both fixes: the two retry tests in ### Convention guards -41 live in `src/server/services/__tests__/no-*.test.ts`: -`no-agent-ground-truth-write`, `no-coerce-boolean-in-api`, `no-direct-shared-module-mock`, +42 live in `src/server/services/__tests__/no-*.test.ts`: +`no-agent-ground-truth-write`, `no-coerce-boolean-in-api`, `no-direct-block-budget-claim-read` (no submit gate may read the `claims.buzzBudget` per-call ceiling directly — every one goes through `blockPerCallBudget`, so a future ceiling decision is made in one place that knows whether the compared value carries the author fee), `no-direct-shared-module-mock`, `no-divergent-active-sales-cap` (SERVER side only: the `model.getActiveSales` parser enforces the id cap, and the chunk size a card surface splits to does not exceed it — it CANNOT see the call site, which is pinned behaviourally by `src/components/Cards/__tests__/useModelSaleBadges.test.ts`, a file in the full unit suite but NOT in `test:lint-rules`, so a `test:lint-rules` run alone does not cover that half; the procedure was rejecting every call from a scrolled feed as an input-validation 400, so no 5xx was recorded and the sale badge simply vanished from the grid), `no-divergent-author-fee-base` (every `recordSpendAttribution` call site must pass the App Blocks author fee the orchestrator's `submitted.cost.base`, never the snapshot and never the gross `buzzAmount`), `no-divergent-can-generate-derivation`, `no-divergent-generation-submit-payload` (the two generation footers must submit the same payload keys — the form-graph lane silently dropped `sourceProvenance`, so its remixes lost the only VERIFIED half of their provenance while the unverified `remixOfId` went through), `no-divergent-model-recency-derivation` (the New/Updated card rule and its day-old cutoff each have one definition — three cards restated them, and when the paid badge took ModelCard's single status slot only that copy knew, so a paid model published minutes ago showed "Paid" on the feed and "New" in the resource picker), `no-divergent-paid-gate-derivation` (the feed and the search index must derive the paid badge from one helper, never two copies of the query), `no-divergent-safetensor-rule` (the coverage view and `checkLoadable` state the checkpoint SafeTensor rule twice and nothing executes the SQL, so the two literals and the checkpoint scoping are pinned textually), `no-doubled-free-slot-noun`, `no-hand-typed-redis-key-constants` (the Redis key-constant ratchet — hand-typed `REDIS_KEYS` in an allowlisted mock had drifted 15 times), `no-io-in-transaction`, `no-job-kind-on-remix-mint`, `no-lint-rules-script-drift`, @@ -210,7 +210,7 @@ fail only in a full-suite run. Five were missing when this was last audited, on wired in then. If the diff adds a guard, check it was wired into the script, and don't treat a green `test:lint-rules` as "all guards passed". -`test:lint-rules` names 46 files today. +`test:lint-rules` names 47 files today. Both numbers and the list are checked by `no-lint-rules-script-drift`, which reads the two phrasings above literally — edit the numbers, not the shapes. diff --git a/CLAUDE.md b/CLAUDE.md index 97551091bd..95b257cb7a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -226,8 +226,8 @@ Use a top-level `import type * as PromClient` — an inline `typeof import('...' **Before widening a mock, check whether the import edge is needed at all.** A failing suite may be telling you the code pulled in a dependency it doesn't want, not that the mock is too narrow, and widening it would hide that. (Bit us twice in one day, Aug 2026, on two branches; one of those three suites was fixed by extracting the helpers into their own module instead.) #### Convention guards run as tests -Several repo conventions are enforced by tests, not by eslint. 41 live in `src/server/services/__tests__/no-*.test.ts` — `no-agent-ground-truth-write`, `no-coerce-boolean-in-api`, -`no-direct-shared-module-mock` (the shared-mock ratchet, see `docs/testing/shared-module-mocks.md`), +Several repo conventions are enforced by tests, not by eslint. 42 live in `src/server/services/__tests__/no-*.test.ts` — `no-agent-ground-truth-write`, `no-coerce-boolean-in-api`, +`no-direct-block-budget-claim-read` (no submit gate may read the `claims.buzzBudget` per-call ceiling directly — every one goes through `blockPerCallBudget`, so a future ceiling decision is made in one place that knows whether the compared value carries the author fee), `no-direct-shared-module-mock` (the shared-mock ratchet, see `docs/testing/shared-module-mocks.md`), `no-divergent-active-sales-cap` (SERVER side only: the `model.getActiveSales` parser enforces the id cap, and the chunk size a card surface splits to does not exceed it — it CANNOT see the call site, which is pinned behaviourally by `src/components/Cards/__tests__/useModelSaleBadges.test.ts`, a file in the full unit suite but NOT in `test:lint-rules`, so a `test:lint-rules` run alone does not cover that half; the procedure was rejecting every call from a scrolled feed as an input-validation 400, so no 5xx was recorded and the sale badge simply vanished from the grid), `no-divergent-author-fee-base` (every `recordSpendAttribution` call site must pass the App Blocks author fee the orchestrator's `submitted.cost.base`, never the snapshot and never the gross `buzzAmount` — the three are indistinguishable positive Buzz integers, so a percentage of the wrong one takes a cut of another creator's licensing fee), `no-divergent-can-generate-derivation` (coverage alone is not canGenerate — the ecosystem must also support the model TYPE, and the pair is composed only in `isGenerationEligible`), `no-divergent-generation-submit-payload` (the two generation footers must submit the same payload keys — the form-graph lane silently dropped `sourceProvenance`, so its remixes lost the only VERIFIED half of their provenance while the unverified `remixOfId` went through), `no-divergent-model-recency-derivation` (the New/Updated card rule and its day-old cutoff each have one definition — three cards restated them, and when the paid badge took ModelCard's single status slot only that copy knew, so a paid model published minutes ago showed "Paid" on the feed and "New" in the resource picker), `no-divergent-paid-gate-derivation` (the feed and the search index must derive the paid badge from one helper, never two copies of the query), `no-divergent-safetensor-rule` (the coverage view and `checkLoadable` state the checkpoint SafeTensor rule twice and nothing executes the SQL, so the two literals and the checkpoint scoping are pinned textually), `no-doubled-free-slot-noun`, `no-hand-typed-redis-key-constants` (the Redis key-constant @@ -271,7 +271,7 @@ was last audited, on 2026-08-24, and were wired in then. **Add a new guard to th you write it**, and don't read a green `test:lint-rules` as "all guards passed" without checking the directory against the script. -`test:lint-rules` names 46 files today. +`test:lint-rules` names 47 files today. The count above, the count in the list, and the list itself are what went stale three times, so `no-lint-rules-script-drift` fails when they disagree with the directory or the script. It reads two exact diff --git a/package.json b/package.json index e9be49eec9..6233316f77 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ "test:packages:run": "vitest run --project '@civitai/*'", "test:apps": "vitest --project 'app:*'", "test:apps:run": "vitest run --project 'app:*'", - "test:lint-rules": "vitest run --project 'unit*' src/server/notifications/__tests__/notification-settings-polarity.test.ts src/server/schema/__tests__/track.addView.schema.test.ts src/server/services/__tests__/hub-filter-parity.test.ts src/server/services/__tests__/poi-checks-strip-benign-phrases.test.ts src/server/services/__tests__/video-leaderboard-badge-staging.test.ts src/server/services/__tests__/no-agent-ground-truth-write.test.ts src/server/services/__tests__/no-coerce-boolean-in-api.test.ts src/server/services/__tests__/no-direct-shared-module-mock.test.ts src/server/services/__tests__/no-divergent-active-sales-cap.test.ts src/server/services/__tests__/no-divergent-author-fee-base.test.ts src/server/services/__tests__/no-divergent-can-generate-derivation.test.ts src/server/services/__tests__/no-divergent-model-recency-derivation.test.ts src/server/services/__tests__/no-divergent-generation-submit-payload.test.ts src/server/services/__tests__/no-divergent-paid-gate-derivation.test.ts src/server/services/__tests__/no-divergent-safetensor-rule.test.ts src/server/services/__tests__/no-doubled-free-slot-noun.test.ts src/server/services/__tests__/no-hand-typed-redis-key-constants.test.ts src/server/services/__tests__/no-io-in-transaction.test.ts src/server/services/__tests__/no-job-kind-on-remix-mint.test.ts src/server/services/__tests__/no-lint-rules-script-drift.test.ts src/server/services/__tests__/no-menu-target-tooltip-nesting.test.ts src/server/services/__tests__/no-module-scope-cache.test.ts src/server/services/__tests__/no-pk-addressed-engagement-write.test.ts src/server/services/__tests__/no-server-infra-in-app-graph.test.ts src/server/services/__tests__/no-sharp-outside-native-project.test.ts src/server/services/__tests__/no-ssr-divergent-media-query.test.ts src/server/services/__tests__/no-stale-moderator-route-probe.test.ts src/server/services/__tests__/no-static-html2canvas-import.test.ts src/server/services/__tests__/no-unbounded-paging-fake.test.ts src/server/services/__tests__/no-unbumped-draft-status-write.test.ts src/server/services/__tests__/no-unfiltered-reaction-metric-sum.test.ts src/server/services/__tests__/no-unguarded-billable-submit.test.ts src/server/services/__tests__/no-unguarded-block-bridge-token.test.ts src/server/services/__tests__/no-unguarded-block-rest-token.test.ts src/server/services/__tests__/no-unguarded-user-text.test.ts src/server/services/__tests__/no-unhydrated-home-block-reactions.test.ts src/server/services/__tests__/no-unloadable-image-fixture.test.ts src/server/services/__tests__/no-unmoderated-blob-retraction.test.ts src/server/services/__tests__/no-unmuteable-comment-processor.test.ts src/server/services/__tests__/no-unpriced-default-model.test.ts src/server/services/__tests__/no-unroled-image-resource-match.test.ts src/server/services/__tests__/no-unscoped-email-verification-exemption.test.ts src/server/services/__tests__/no-untruthy-query-gate.test.ts src/server/services/__tests__/no-unverified-provenance-write.test.ts src/server/services/__tests__/no-unwrapped-knob-rotation.test.ts src/server/services/__tests__/no-wholesale-module-mock.test.ts", + "test:lint-rules": "vitest run --project 'unit*' src/server/notifications/__tests__/notification-settings-polarity.test.ts src/server/schema/__tests__/track.addView.schema.test.ts src/server/services/__tests__/hub-filter-parity.test.ts src/server/services/__tests__/poi-checks-strip-benign-phrases.test.ts src/server/services/__tests__/video-leaderboard-badge-staging.test.ts src/server/services/__tests__/no-agent-ground-truth-write.test.ts src/server/services/__tests__/no-coerce-boolean-in-api.test.ts src/server/services/__tests__/no-direct-shared-module-mock.test.ts src/server/services/__tests__/no-direct-block-budget-claim-read.test.ts src/server/services/__tests__/no-divergent-active-sales-cap.test.ts src/server/services/__tests__/no-divergent-author-fee-base.test.ts src/server/services/__tests__/no-divergent-can-generate-derivation.test.ts src/server/services/__tests__/no-divergent-model-recency-derivation.test.ts src/server/services/__tests__/no-divergent-generation-submit-payload.test.ts src/server/services/__tests__/no-divergent-paid-gate-derivation.test.ts src/server/services/__tests__/no-divergent-safetensor-rule.test.ts src/server/services/__tests__/no-doubled-free-slot-noun.test.ts src/server/services/__tests__/no-hand-typed-redis-key-constants.test.ts src/server/services/__tests__/no-io-in-transaction.test.ts src/server/services/__tests__/no-job-kind-on-remix-mint.test.ts src/server/services/__tests__/no-lint-rules-script-drift.test.ts src/server/services/__tests__/no-menu-target-tooltip-nesting.test.ts src/server/services/__tests__/no-module-scope-cache.test.ts src/server/services/__tests__/no-pk-addressed-engagement-write.test.ts src/server/services/__tests__/no-server-infra-in-app-graph.test.ts src/server/services/__tests__/no-sharp-outside-native-project.test.ts src/server/services/__tests__/no-ssr-divergent-media-query.test.ts src/server/services/__tests__/no-stale-moderator-route-probe.test.ts src/server/services/__tests__/no-static-html2canvas-import.test.ts src/server/services/__tests__/no-unbounded-paging-fake.test.ts src/server/services/__tests__/no-unbumped-draft-status-write.test.ts src/server/services/__tests__/no-unfiltered-reaction-metric-sum.test.ts src/server/services/__tests__/no-unguarded-billable-submit.test.ts src/server/services/__tests__/no-unguarded-block-bridge-token.test.ts src/server/services/__tests__/no-unguarded-block-rest-token.test.ts src/server/services/__tests__/no-unguarded-user-text.test.ts src/server/services/__tests__/no-unhydrated-home-block-reactions.test.ts src/server/services/__tests__/no-unloadable-image-fixture.test.ts src/server/services/__tests__/no-unmoderated-blob-retraction.test.ts src/server/services/__tests__/no-unmuteable-comment-processor.test.ts src/server/services/__tests__/no-unpriced-default-model.test.ts src/server/services/__tests__/no-unroled-image-resource-match.test.ts src/server/services/__tests__/no-unscoped-email-verification-exemption.test.ts src/server/services/__tests__/no-untruthy-query-gate.test.ts src/server/services/__tests__/no-unverified-provenance-write.test.ts src/server/services/__tests__/no-unwrapped-knob-rotation.test.ts src/server/services/__tests__/no-wholesale-module-mock.test.ts", "test:component": "node scripts/test-component-run.mjs", "test:component:watch": "vitest --project component", "test:geometry": "vitest run --project geometry", diff --git a/src/server/middleware/block-scope.middleware.ts b/src/server/middleware/block-scope.middleware.ts index e9d66067ac..07b8581a59 100644 --- a/src/server/middleware/block-scope.middleware.ts +++ b/src/server/middleware/block-scope.middleware.ts @@ -54,6 +54,16 @@ export interface BlockTokenClaims { blockInstanceId: string; ctx: Record; scopes: string[]; + /** + * The per-call generation ceiling the app declared (`page.buzzBudgetPerGen` / + * `settings.buzz_budget_per_gen`), clamped by the minting resolver. + * + * 🔴 A SUBMIT GATE READS IT ONLY THROUGH `blockPerCallBudget` — see that + * function for what the indirection is and is not doing today. The read + * surfaces that merely REPORT the number to the block (`/api/v1/blocks/me`, + * `blocks.getMyViewer`) project it directly and are ledgered as such in + * `src/server/services/__tests__/no-direct-block-budget-claim-read.test.ts`. + */ buzzBudget?: number; /** * AUTHORITATIVE color-domain maturity ceiling (bitwise browsing-level flag, @@ -100,6 +110,68 @@ export interface BlockTokenClaims { reviewRunForReal?: boolean; } +/** + * THE per-call Buzz ceiling a submit gate compares against. + * + * ── WHAT IT DOES TODAY: NOTHING A CALLER COULD NOT DO INLINE ──────────────── + * It returns `claims.buzzBudget`, or 0 when no budget was minted. BOTH values of + * `pricesAuthorFee` return that same number. Routing the four submit gates + * through it is a no-op on behaviour, and that is the entire intent: it + * consolidates four copies of one comparison without altering any of them. + * + * ── WHY IT EXISTS AT ALL ──────────────────────────────────────────────────── + * The four gates spell the same comparison and are NOT interchangeable. Two of + * them add the per-generation author fee into the value they compare; two do not, + * because neither has a pre-submit `cost.base` to price a fee from (that split is + * documented at `src/server/services/blocks/author-fee-charge.service.ts` and + * ledgered in `src/server/services/__tests__/no-divergent-author-fee-base.test.ts`). + * Any future change to the ceiling is correct for at most one of those two + * populations, so it has to be made somewhere that knows which gate is which. + * This is that place; `pricesAuthorFee` is how each gate declares which it is. + * + * 🔴 THE FLAG IS CLASSIFICATION ONLY, AND HAS NO EFFECT. It changes no value, no + * branch reads it, and there is no ceiling decision behind it yet. A gate passing + * the "wrong" one is therefore not a defect today, because nothing consumes it. + * Do not read its presence as evidence that a difference exists. It is pinned + * against the fee call sites by + * `src/server/services/__tests__/no-direct-block-budget-claim-read.test.ts` so the + * classification cannot drift out of step with the fee before it becomes + * load-bearing — which it does the moment any branch reads it. + * + * 🔴 A RAISED CEILING WAS PROPOSED HERE AND WITHDRAWN. Minting `buzzBudget` ABOVE + * the declared ceiling so a fee fits underneath it is sound only where the fee is + * inside the compared value, and it was unsound in two ways at once. On the two + * fee-free gates the number that clears the gate is the number reserved and + * billed, so a raised ceiling spends real viewer Buzz above the ceiling the app's + * manifest declared. And on a fee-pricing gate it is equally unsound whenever the + * fee prices to zero — which it does for at least one generation type today — so + * the headroom is generation headroom no fee ever consumes. The manifest schema + * describes that declared number to authors as a safety ceiling against a + * compromised app draining the viewer's Buzz, so exceeding it is the one thing it + * must not do. + * + * So a future raised ceiling has to arrive as a SEPARATE claim whose name says it + * is granted, read only by a gate that prices the fee into its compared value — + * never by widening what `buzzBudget` means, which would hand the money-unsafe + * value to every gate written the obvious way. + * + * Returns 0 when no budget was minted, so a caller that skipped the + * `typeof claims.buzzBudget !== 'number'` pre-check still fails CLOSED. + */ +export function blockPerCallBudget( + claims: Pick, + // Classification only, read by no branch today — see the note above. Kept in + // the signature so each gate declares its population and the ledger can pin + // that declaration against the fee call sites. The directive must stay on the + // line immediately above the parameter: anything between them and it silently + // applies to the comment instead. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + opts: { pricesAuthorFee: boolean } +): number { + if (typeof claims.buzzBudget !== 'number') return 0; + return claims.buzzBudget; +} + export type BlockScopedNextApiRequest = NextApiRequest & { blockClaims?: BlockTokenClaims; }; diff --git a/src/server/routers/__tests__/blocks.router.scopeEnforcement.test.ts b/src/server/routers/__tests__/blocks.router.scopeEnforcement.test.ts index 93fb5e8866..095e22b029 100644 --- a/src/server/routers/__tests__/blocks.router.scopeEnforcement.test.ts +++ b/src/server/routers/__tests__/blocks.router.scopeEnforcement.test.ts @@ -139,7 +139,13 @@ vi.mock('~/server/utils/block-gen-idempotency', async (importActual) => { vi.mock('~/server/services/blocks/user-app-surface.service', () => ({ recordScopeInvocation: vi.fn(async () => undefined), })); -vi.mock('~/server/middleware/block-scope.middleware', () => ({ +// Spread the ORIGINAL rather than replacing the module: `blocks.router.ts` also +// imports `blockPerCallBudget` from here, and every submit gate's budget +// comparison goes through it. Stubbing the module wholesale drops that export, +// so each gate would compare against `undefined` — and `x > undefined` is +// always false, i.e. the budget gate silently stops rejecting anything. +vi.mock('~/server/middleware/block-scope.middleware', async (importOriginal) => ({ + ...(await importOriginal>()), verifyBlockToken: mockVerifyBlockToken, parseSubjectUserId: (...args: unknown[]) => mockParseSubjectUserId(...args), })); diff --git a/src/server/routers/__tests__/blocks.router.workflow.test.ts b/src/server/routers/__tests__/blocks.router.workflow.test.ts index 9d7a171f1d..4a01e83408 100644 --- a/src/server/routers/__tests__/blocks.router.workflow.test.ts +++ b/src/server/routers/__tests__/blocks.router.workflow.test.ts @@ -227,7 +227,13 @@ vi.mock('~/server/services/blocks/user-app-surface.service', () => ({ recordScopeInvocation: vi.fn(async () => undefined), })); -vi.mock('~/server/middleware/block-scope.middleware', () => ({ +// Spread the ORIGINAL rather than replacing the module: `blocks.router.ts` also +// imports `blockPerCallBudget` from here, and every submit gate's budget +// comparison goes through it. Stubbing the module wholesale drops that export, +// so each gate would compare against `undefined` — and `x > undefined` is +// always false, i.e. the budget gate silently stops rejecting anything. +vi.mock('~/server/middleware/block-scope.middleware', async (importOriginal) => ({ + ...(await importOriginal>()), verifyBlockToken: mockVerifyBlockToken, parseSubjectUserId: (...args: unknown[]) => mockParseSubjectUserId(...args), })); diff --git a/src/server/routers/blocks.router.ts b/src/server/routers/blocks.router.ts index 0208abf9aa..ac33b53a21 100644 --- a/src/server/routers/blocks.router.ts +++ b/src/server/routers/blocks.router.ts @@ -12,6 +12,7 @@ import { FORGEJO_ORG } from '~/server/services/blocks/forgejo.service'; import { logToAxiom } from '~/server/logging/client'; import { getOrchestratorToken } from '~/server/orchestrator/get-orchestrator-token'; import { + blockPerCallBudget, parseSubjectUserId, type BlockTokenClaims, } from '~/server/middleware/block-scope.middleware'; @@ -5500,7 +5501,7 @@ export const blocksRouter = router({ // record. There are FOUR below, all returning this same whatIf `cost` and // no workflow id the caller could poll to learn the answer later: // - // 1. insufficient per-call budget (`cost > claims.buzzBudget`) + // 1. insufficient per-call budget (`cost > perCallBudget`) // 2. per-user daily / review-session Buzz cap (`total > buzzCap`) // 3. per-app aggregate spend + velocity cap (G8, `!appSpend.allowed`) // 4. dev-tunnel per-session spend backstop (F4, `!reserved.allowed`) @@ -5569,7 +5570,12 @@ export const blocksRouter = router({ // reading the orchestrator's own number, not one this line inflated. const quotedGenerationBuzz = whatIfResult.cost?.total ?? 0; const cost = quotedGenerationBuzz + reservedAuthorFeeBuzz; - if (cost > claims.buzzBudget) { + // `pricesAuthorFee: true` — `cost` carries the author fee (line above). + // The helper returns `claims.buzzBudget` either way, so this comparison is + // unchanged; the flag classifies the gate, it does not price it. See + // `blockPerCallBudget`. + const perCallBudget = blockPerCallBudget(claims, { pricesAuthorFee: true }); + if (cost > perCallBudget) { return { snapshot: { // Non-empty sentinel: the block SDK validator drops empty-workflowId @@ -5579,7 +5585,9 @@ export const blocksRouter = router({ workflowId: 'failed', status: 'failed' as const, cost: { total: cost }, - error: `insufficient buzz budget: estimate ${cost} exceeds budget ${claims.buzzBudget}`, + // Quotes the ceiling that was actually compared, so the sentence + // stays true if that ceiling ever stops being the raw claim. + error: `insufficient buzz budget: estimate ${cost} exceeds budget ${perCallBudget}`, // Additive + omitted when empty, exactly like every other snapshot // site, so this reply is byte-identical whenever nothing was // substituted. @@ -8128,13 +8136,19 @@ async function submitCustomComfyWorkflow(opts: { // `cost > buzzBudget` gate. Because the timeout caps the job at `maxBuzz` and we // require `maxBuzz <= buzzBudget`, the per-call budget CANNOT be exceeded. // Deterministic, no orchestrator round-trip. - if (ceiling > claims.buzzBudget) { + // 🔴 `pricesAuthorFee: false` — THIS PATH CHARGES NO AUTHOR FEE (no pre-submit + // `cost.base` to price one from), so `ceiling` is a raw generation price. The + // helper returns `claims.buzzBudget` either way, so this comparison is + // unchanged; the flag records that a raised ceiling must never reach here. See + // `blockPerCallBudget`. + const perCallBudget = blockPerCallBudget(claims, { pricesAuthorFee: false }); + if (ceiling > perCallBudget) { return { snapshot: { workflowId: 'failed', status: 'failed' as const, cost: { total: ceiling }, - error: `insufficient buzz budget: recipe ceiling ${ceiling} exceeds budget ${claims.buzzBudget}`, + error: `insufficient buzz budget: recipe ceiling ${ceiling} exceeds budget ${perCallBudget}`, }, }; } @@ -9345,13 +9359,16 @@ async function submitStepWorkflow(opts: { // (1) Pre-submit gate against the token's per-call budget — now enforced // against the ORCHESTRATOR'S OWN NUMBER, not a declared constant. - if (reserveBuzz > claims.buzzBudget) { + // `pricesAuthorFee: true` — `reserveBuzz` carries the author fee (line above). + // Classification only; the helper returns `claims.buzzBudget` either way. + const perCallBudget = blockPerCallBudget(claims, { pricesAuthorFee: true }); + if (reserveBuzz > perCallBudget) { return { snapshot: { workflowId: 'failed', status: 'failed' as const, cost: { total: reserveBuzz }, - error: `insufficient buzz budget: step price ${reserveBuzz} exceeds budget ${claims.buzzBudget}`, + error: `insufficient buzz budget: step price ${reserveBuzz} exceeds budget ${perCallBudget}`, }, }; } @@ -10164,13 +10181,21 @@ async function submitPassThroughStepWorkflow(opts: { const ceiling = Math.max(body.maxBuzz, quotedBuzz ?? body.maxBuzz); // (1) STATIC pre-submit gate against the token's per-call budget. - if (ceiling > claims.buzzBudget) { + // + // 🔴 `pricesAuthorFee: false` — this path charges no author fee, and `ceiling` + // is not merely compared here: it is what `reserveBlockBuzzSpendForClaims` + // reserves below and what the terminal settle bills against. That is why the + // classification matters even though it changes nothing today — a ceiling + // raised above what the app declared would be BILLED here, not just compared. + // The helper returns `claims.buzzBudget` either way. See `blockPerCallBudget`. + const perCallBudget = blockPerCallBudget(claims, { pricesAuthorFee: false }); + if (ceiling > perCallBudget) { return { snapshot: { workflowId: 'failed', status: 'failed' as const, cost: { total: ceiling }, - error: `insufficient buzz budget: step ceiling ${ceiling} exceeds budget ${claims.buzzBudget}`, + error: `insufficient buzz budget: step ceiling ${ceiling} exceeds budget ${perCallBudget}`, }, }; } diff --git a/src/server/services/__tests__/no-direct-block-budget-claim-read.test.ts b/src/server/services/__tests__/no-direct-block-budget-claim-read.test.ts new file mode 100644 index 0000000000..cb3ccfc271 --- /dev/null +++ b/src/server/services/__tests__/no-direct-block-budget-claim-read.test.ts @@ -0,0 +1,412 @@ +import { readFileSync, readdirSync } from 'fs'; +import path from 'path'; +import { describe, expect, it } from 'vitest'; + +/** + * NO SUBMIT GATE READS `claims.buzzBudget` DIRECTLY — every one of them goes + * through `blockPerCallBudget`. + * + * ── WHY THE INDIRECTION IS WORTH A GUARD WHEN IT CHANGES NO VALUE ─────────── + * `blockPerCallBudget` returns `claims.buzzBudget` on both of its + * `pricesAuthorFee` classifications, so routing the gates through it alters + * nothing today. What it buys is a single place where a future ceiling decision + * can be made KNOWING WHICH GATE IS ASKING — and that distinction is a money + * question, because the four gates are not interchangeable: + * + * - two of them add the per-generation author fee into the value they compare; + * - two do not, and on the pass-through path the value that clears the gate is + * the value reserved and the value the terminal settle BILLS. + * + * A ceiling raised above what the app's manifest declared is therefore spendable + * viewer Buzz on the second pair, and the manifest schema describes that declared + * number to authors as a safety ceiling against a compromised app. A gate that + * reads the claim directly is outside whatever decision gets made here, and that + * is the drift this file exists to make impossible. + * + * ── WHY IT IS STRUCTURAL RATHER THAN SPELLED, AND WHAT THAT COST ──────────── + * 🔴 THE PREVIOUS VERSION OF THIS LEDGER WAS SPELLED, AND THE EXACT DEFECT IT + * NAMED SURVIVED IT. It asserted `router.match(/> claims\.buzzBudget\b/g)` was + * empty — a check on ONE literal spelling of one comparison operator in ONE file. + * This mutant passed the entire suite: + * + * - if (ceiling > perCallBudget) + * + if (ceiling > (claims.buzzBudget ?? perCallBudget)) + * + * on the fee-free pass-through gate, which then reserves and bills against the + * raw claim. The parenthesis alone defeated the regex; so would `>=`, `<`, `??`, + * or hoisting the claim into a local first. + * + * So this inverts the test: EVERY occurrence of `claims.buzzBudget` in production + * source is a violation unless it matches one of the ALLOWED forms enumerated + * below, each carrying its reason. A new spelling is a violation by default + * rather than by enumeration, which is the only direction that can be complete. + * + * ── WHY THE CORPUS IS THE WHOLE TREE ──────────────────────────────────────── + * 🔴 THE PREVIOUS VERSION READ `blocks.router.ts` AND NOTHING ELSE, so a budget + * gate added in any other module was invisible to every count it took. Every + * production `.ts`/`.tsx` under `src/` is scanned here. Test files are excluded: + * a fixture is free to spell a claims bag any way it likes, and a ledger that + * fails on someone else's test data is a ledger people delete. + * + * ── WHY IT IS WHITESPACE-INDEPENDENT ──────────────────────────────────────── + * 🔴 THE PREVIOUS VERSION MATCHED A SINGLE-LINE SPELLING OF THE CALL, so a fifth + * gate wrapped across lines by prettier (printWidth 100) left its counts reading + * 2 and 2 and passed green. Call sites here are found by balanced-paren + * extraction and normalised before matching, so line breaks and a trailing comma + * cannot hide one. + * + * ── WHY THIS FILE LIVES HERE ──────────────────────────────────────────────── + * `no-lint-rules-script-drift` scans only `src/server/services/__tests__` for + * only this `no-*.test.ts` name shape. A guard of this class parked beside the + * module it protects is invisible to that ratchet and is not run by the fast + * `pnpm run test:lint-rules` selector — it would surface only in a full unit + * suite, minutes later, in a file nobody was looking at. + */ + +const SRC = path.resolve(__dirname, '../../..'); + +/** The claim this ledger governs. */ +const CLAIM = 'claims.buzzBudget'; + +/** + * Blank out every comment, preserving byte offsets and newlines so a violation's + * line number survives. + * + * 🔴 PROSE MUST NOT BE ABLE TO TURN THIS RED. Half the files in the corpus + * legitimately discuss `claims.buzzBudget` in a doc comment — this file does too + * — and a ledger a comment can fail is a ledger someone loosens until it cannot + * fail at all. + * + * A backslash always consumes the following character, in every context, so an + * escaped slash inside a regex literal (`/\/\//`) cannot be mistaken for the + * start of a line comment. That is the one case where a naive stripper would eat + * real code — the fail-OPEN direction, which is why it is handled rather than + * assumed. The positive controls below are what prove the stripper did not eat + * anything: they assert the real allowed forms are still FOUND after stripping. + */ +function blankComments(source: string): string { + const out = source.split(''); + let i = 0; + const blank = (from: number, to: number) => { + for (let j = from; j < to && j < out.length; j += 1) { + if (out[j] !== '\n') out[j] = ' '; + } + }; + while (i < source.length) { + const c = source[i]; + if (c === '\\') { + i += 2; + continue; + } + if (c === "'" || c === '"' || c === '`') { + const quote = c; + i += 1; + while (i < source.length) { + if (source[i] === '\\') i += 2; + else if (source[i] === quote) { + i += 1; + break; + } else i += 1; + } + continue; + } + if (c === '/' && source[i + 1] === '/') { + const end = source.indexOf('\n', i); + blank(i, end === -1 ? source.length : end); + i = end === -1 ? source.length : end; + continue; + } + if (c === '/' && source[i + 1] === '*') { + const end = source.indexOf('*/', i + 2); + const stop = end === -1 ? source.length : end + 2; + blank(i, stop); + i = stop; + continue; + } + i += 1; + } + return out.join(''); +} + +/** + * THE ALLOWED FORMS — the complete list of ways production code may name this + * claim. Anything else is a violation, including a shape that is obviously + * harmless: the point is that the SET is closed, so a new reader has to add an + * entry and say why rather than inventing a spelling that slips past a pattern. + * + * `files` scopes a form to the modules entitled to it, so the helper's own return + * cannot license the same line inside a gate. + */ +type AllowedForm = { + name: string; + /** Matched against comment-blanked source; `\s+` tolerates prettier wrapping. */ + re: RegExp; + /** Repo-relative paths (from `src/`) allowed to carry this form. */ + files: string[]; + /** How many occurrences exist today, across all allowed files. */ + count: number; + why: string; +}; + +const ALLOWED_FORMS: AllowedForm[] = [ + { + name: 'presence / positivity pre-check', + // `typeof claims.buzzBudget !== 'number'`, optionally with the + // `|| claims.buzzBudget <= 0` positivity leg of the same condition. + re: /typeof\s+claims\.buzzBudget\s*!==\s*'number'(?:\s*\|\|\s*claims\.buzzBudget\s*<=\s*0)?/g, + files: ['server/routers/blocks.router.ts', 'server/middleware/block-scope.middleware.ts'], + count: 6, + why: + 'Asks whether a budget was minted at all, and rejects a non-positive one. It compares ' + + 'the claim against no price, so it cannot be the site where a ceiling decision is ' + + 'skipped — and every gate needs it before the comparison the helper owns.', + }, + { + name: "the helper's own return", + re: /return\s+claims\.buzzBudget;/g, + files: ['server/middleware/block-scope.middleware.ts'], + count: 1, + why: + '`blockPerCallBudget` is the one function entitled to read the claim for a gate. This ' + + 'is the read every gate is routed through.', + }, + { + name: 'the single mint-site write', + re: /claims\.buzzBudget\s*=\s*input\.buzzBudget;/g, + files: ['server/services/block-token.service.ts'], + count: 1, + why: + 'The one place the claim is WRITTEN. `BlockTokenService.sign` is the only block-token ' + + 'signer, so pinning this at exactly one occurrence is the single-writer property: a ' + + 'second mint computing a budget inline would land here as a violation rather than as a ' + + 'silent second definition of what the claim means. It is also the site any future ' + + 'ceiling change would be tempted to edit — which is why it is enumerated rather than ' + + 'left to a pattern.', + }, + { + name: 'read-surface projection', + re: /buzzBudget:\s*claims\.buzzBudget\s*\?\?\s*null/g, + files: ['server/routers/blocks.router.ts', 'pages/api/v1/blocks/me.ts'], + count: 2, + why: + 'The two doors that REPORT the number to the block (`blocks.getMyViewer` and ' + + '`/api/v1/blocks/me`) — a projection onto a response body, not a comparison against a ' + + 'price. They are pinned to each other by ' + + '`src/server/routers/__tests__/blocks.router.me-parity.test.ts`. If the minted claim ever ' + + 'stops being the number an app may price a generation at, these are the two sites that ' + + 'have to be revisited in the same commit.', + }, +]; + +/** Every production `.ts`/`.tsx` under `src/`, tests excluded. */ +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name === 'node_modules' || entry.name === '.next') continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) walk(full, out); + else if ( + entry.isFile() && + /\.tsx?$/.test(entry.name) && + !/\.test\.tsx?$/.test(entry.name) && + !full.includes(`${path.sep}__tests__${path.sep}`) + ) + out.push(full); + } + return out; +} + +const rel = (f: string) => path.relative(SRC, f).split(path.sep).join('/'); +const sourceFiles = walk(SRC); + +/** `` for every file that names the claim at all. */ +const scanned = sourceFiles + .map((f) => ({ file: rel(f), code: blankComments(readFileSync(f, 'utf8')) })) + .filter((f) => f.code.includes(CLAIM)); + +/** + * Remove every ALLOWED occurrence from a file, preserving offsets, and return + * what is left over plus a per-form tally of what was removed. + */ +function residualReads(file: string, code: string) { + let remaining = code; + const removed: Record = {}; + for (const form of ALLOWED_FORMS) { + if (!form.files.includes(file)) continue; + remaining = remaining.replace(form.re, (m) => { + removed[form.name] = (removed[form.name] ?? 0) + 1; + return m.replace(/[^\n]/g, ' '); + }); + } + + const violations: string[] = []; + for (let at = remaining.indexOf(CLAIM); at !== -1; at = remaining.indexOf(CLAIM, at + 1)) { + const line = remaining.slice(0, at).split('\n').length; + const text = remaining.split('\n')[line - 1].trim(); + violations.push(`${file}:${line} ${text}`); + } + return { violations, removed }; +} + +/** Every `blockPerCallBudget(...)` call site in the corpus, braces balanced. */ +function perCallBudgetSites(code: string): string[] { + const sites: string[] = []; + const opener = 'blockPerCallBudget('; + for (let start = code.indexOf(opener); start !== -1; start = code.indexOf(opener, start + 1)) { + // Skip the declaration itself — `export function blockPerCallBudget(`. + if (/function\s+$/.test(code.slice(Math.max(0, start - 20), start))) continue; + let depth = 0; + let i = start + opener.length - 1; + for (; i < code.length; i += 1) { + if (code[i] === '(') depth += 1; + else if (code[i] === ')') { + depth -= 1; + if (depth === 0) break; + } + } + // Normalised so prettier line-wrapping cannot change what a site looks like. + sites.push(code.slice(start, i + 1).replace(/\s+/g, ' ')); + } + return sites; +} + +const callSitesByFile = sourceFiles + .map((f) => ({ file: rel(f), sites: perCallBudgetSites(blankComments(readFileSync(f, 'utf8'))) })) + .filter((f) => f.sites.length > 0); + +describe('the instrument itself', () => { + it('walks the tree and reaches the directories this ledger reasons about', () => { + // A walk that reached one top-level directory would make every assertion + // below vacuous rather than red, and a token floor like 500 would not see it. + expect(sourceFiles.length).toBeGreaterThan(3000); + for (const dir of ['server/', 'pages/', 'components/']) { + expect(sourceFiles.some((f) => rel(f).startsWith(dir))).toBe(true); + } + }); + + it('blanks comments without eating code (positive + negative control)', () => { + const sample = [ + 'const a = claims.buzzBudget; // claims.buzzBudget in a line comment', + '/* claims.buzzBudget in a block comment */', + 'const slash = /\\/\\//; const b = claims.buzzBudget;', + "const s = '// claims.buzzBudget in a string';", + ].join('\n'); + const blanked = blankComments(sample); + + // NEGATIVE CONTROL — the two comment occurrences are gone… + expect(blanked).not.toContain('in a line comment'); + expect(blanked).not.toContain('in a block comment'); + // …POSITIVE CONTROL — and three survive: the two real reads, including the + // one after a regex literal containing an escaped `//` (the case a naive + // stripper eats silently), plus the one inside a string literal. Strings are + // skipped for COMMENT DETECTION only, never erased, so a claim named inside + // one would be reported — the fail-CLOSED direction. + expect((blanked.match(/claims\.buzzBudget/g) ?? []).length).toBe(3); + // Offsets are preserved, so a reported line number is the real one. + expect(blanked.split('\n')).toHaveLength(sample.split('\n').length); + expect(blanked.length).toBe(sample.length); + }); + + it('reports a violation for a read no allowed form covers (negative control)', () => { + // A ledger nobody has watched go red is a claim about its own regexes. This + // is the F2 mutant's exact shape, and a hoisted local, fed in synthetically. + const mutant = [ + " if (typeof claims.buzzBudget !== 'number' || claims.buzzBudget <= 0) return;", + ' if (ceiling > (claims.buzzBudget ?? perCallBudget)) return;', + ' const b = claims.buzzBudget;', + ].join('\n'); + const { violations } = residualReads('server/routers/blocks.router.ts', mutant); + expect(violations).toHaveLength(2); + expect(violations[0]).toContain('server/routers/blocks.router.ts:2'); + expect(violations[1]).toContain('server/routers/blocks.router.ts:3'); + }); + + it('finds the call sites it counts (positive control)', () => { + expect(callSitesByFile.length).toBeGreaterThan(0); + expect(callSitesByFile.flatMap((f) => f.sites).length).toBeGreaterThan(0); + }); +}); + +describe('no production code reads the per-call budget claim outside the allowed forms', () => { + it('every occurrence in the tree matches an allowed form', () => { + const violations = scanned.flatMap((f) => residualReads(f.file, f.code).violations); + expect( + violations, + 'These read `claims.buzzBudget` in a shape no ALLOWED_FORMS entry covers. A submit gate ' + + 'must compare against `blockPerCallBudget(claims, { pricesAuthorFee })` instead — that ' + + 'is the one place a ceiling decision can be made knowing whether the compared value ' + + 'carries the author fee. If the read is genuinely not a gate, add a form above WITH ITS ' + + 'REASON rather than widening an existing pattern.' + ).toEqual([]); + }); + + it('every allowed form is still exercised, at its stated count', () => { + // Fails on SHRINK as well as growth: a form whose occurrences vanished is a + // stale exemption, and — more importantly — a form that silently stopped + // matching would make the test above pass by removing nothing. + const tally: Record = {}; + for (const f of scanned) { + for (const [name, n] of Object.entries(residualReads(f.file, f.code).removed)) { + tally[name] = (tally[name] ?? 0) + n; + } + } + for (const form of ALLOWED_FORMS) { + expect( + tally[form.name] ?? 0, + `allowed form "${form.name}" — update its count or drop it` + ).toBe(form.count); + } + }); + + it('names no file that no longer carries the form it was allowed for', () => { + const stale: string[] = []; + for (const form of ALLOWED_FORMS) { + for (const file of form.files) { + const entry = scanned.find((f) => f.file === file); + form.re.lastIndex = 0; + if (!entry || !form.re.test(entry.code)) stale.push(`${form.name} → ${file}`); + } + } + expect(stale, 'drop these from ALLOWED_FORMS[].files').toEqual([]); + }); +}); + +describe('the four submit gates are routed through one helper', () => { + const sites = callSitesByFile.flatMap((f) => f.sites); + + it('every call site declares which kind of gate it is', () => { + // 🔴 A GATE WITHOUT THE FLAG IS THE DEFECT THIS PINS. The parameter is + // classification only and no branch reads it today, so a missing one is a + // type error rather than a money bug right now — but it becomes load-bearing + // the moment any ceiling decision lands behind it, and by then nobody will + // re-derive which population each gate belongs to. + for (const site of sites) expect(site).toMatch(/pricesAuthorFee:\s*(true|false)\b/); + }); + + it('the fee-pricing and fee-free populations are exactly the two known sets', () => { + const pricing = sites.filter((s) => /pricesAuthorFee:\s*true\b/.test(s)); + const feeFree = sites.filter((s) => /pricesAuthorFee:\s*false\b/.test(s)); + + // txt2img submit and the registry step price a fee; customComfy/recipe and + // the pass-through step do not. Adding a gate means deciding which it is. + expect(pricing).toHaveLength(2); + expect(feeFree).toHaveLength(2); + expect(sites).toHaveLength(4); + }); + + it('the fee-pricing population agrees with the fee call sites', () => { + // 🔴 THE RELATIONSHIP, not a component: a path that GROWS a fee without + // flipping its flag, or flips a flag without wiring a fee, moves exactly one + // of these two numbers. Both are read from the same file so neither can be + // satisfied by the other's population. + const router = blankComments( + readFileSync(path.join(SRC, 'server/routers/blocks.router.ts'), 'utf8') + ); + const feeQuotes = router.match(/quoteBlockAuthorFee\(\{/g) ?? []; + expect(feeQuotes.length, 'no fee quotes found — the matcher is wrong').toBeGreaterThan(0); + expect(feeQuotes).toHaveLength(2); + expect( + (router.match(/pricesAuthorFee:\s*true\b/g) ?? []).length, + 'a submit path priced a fee without flipping its gate flag, or the reverse' + ).toBe(feeQuotes.length); + }); +});