mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
fix(blocks): narrow the dev-token exemption so a suspended app stops driving the bridge for 4h (#4980)
* fix(blocks): the dev-token exemption no longer skips the approved check for every dev token
The App Blocks approved-status predicate short-circuited on `claims.dev === true`,
so a moderator suspension left already-minted dev tokens driving both halves of the
runtime — the 15 tRPC bridge procedures and every `withBlockScope` REST route — for
the remaining life of the token. Dev tokens live 14400s against a 900s default, so
that window was 16x every other token's, on the class with the widest scopes.
The `dev` claim is stamped unconditionally by `signDevScopedPageToken`, which six
mint paths reach. Three of them must run a non-approved app; three must not. Keying
on the bare boolean exempted all six, and the docblock justified it by listing belts
the guard re-checked none of — ownership, an active dev tunnel, the author flags,
the approved-scope clamp.
The predicate now separates them:
- a signed `reviewRunForReal` claim short-circuits ahead of the read, which is the
moderator review sandbox and the one population that must run a non-approved app
without owning it;
- no backing row means nothing to be approved, covering the synthetic-id mints;
- a REAL row that is not approved is exempt only when the subject IS the app's
owner AND that owner has an ACTIVE dev tunnel for the slug — the two preconditions
the owner-dev-tunnel mint enforces, re-derived rather than assumed;
- anything else with a real, non-approved row is refused, which is the `dev:live`
token whose mint required approval and that has simply outlived it.
Ownership comes from one extra selected column on the lookup this path already
performs. The tunnel check is reached only on the dev + real-row + not-approved
path, so no approved app and no non-dev token pays for it.
Revocation is untouched — it was never exempted — and the 4h lifetime is unchanged.
Refs clawgate #571.
* fix(blocks): resolve the dev-token owner in the branch, not as a second query on every request
Findings from this repo's five-lane review, applied. Three of them were wrong
in ways a green suite could not show.
PERF — the owner was read as `app: { select: { userId: true } }` on the row
lookup, and two comments asserted that was "a join on the FK, not a second
query". It is not: the schema enables only `previewFeatures = ["metrics"]`, so
with no `relationJoins` Prisma resolves a nested relation with another round
trip, and because `appId` is a REQUIRED relation that query cannot be skipped on
an empty FK set. It would have fired for every token whose row exists — every
bridge call including the timer-driven `pollWorkflow`, and every block-JWT REST
request — to read a column only the dev branch consults. `claims.appId` IS the
`OauthClient.id`, so the lookup moves into the branch as the same primary-key
read, issued only when needed. The row read returns to `select: { status: true }`
and a test now pins that literal.
TESTS — moving the `dev` check behind the row read silently made the existing
`dev=%p is NOT exempt` table VACUOUS. Its fixture carried no owner, so the
mutant `!claims.dev` fell through to the ownership guard and was refused there:
the table stayed green while the guard it claims to pin never executed. The
repair is in the fixture — clear every other reason to refuse, so the verdict is
attributable to the `dev` comparison alone. Six mutants now each die to their own
test, including this one.
NARROWING — `reviewRunForReal` was exempting without requiring `dev`. Every mint
stamps both, but `sign` accepts the field independently, and a bypass keyed on one
signed boolean without narrowing it is the defect this change exists to fix; alone
it would have been WIDER than the blanket exemption it replaced.
POSTURE — `getActiveDevTunnel` attaches its `.catch()` to the result of
`sysRedis.get(...)`, so a synchronous client throw escapes it, as can the dynamic
import. Unwrapped that becomes a 503 blamed on the replica read. Now wrapped, so
the fail-closed posture is written rather than inherited.
REUSE — `subjectForUserId` already existed in `block-revocation.service` under a
docblock claiming to be "THE ONE PLACE this format is written on the WRITE side".
It was not — the mint open-coded the same template — and this change would have
been a third copy, each pinned by its own literal so no test could see them
diverge. Moved to a zero-import leaf both now use, re-exported so no importer
changes.
Plus the written half: the population table the code referred to now exists as a
table, the `ai:write:budgeted` containment claim is scoped to the population it
is actually true of, and the two sibling resolvers each carry a cross-reference
to the other two.
Refs clawgate #571.
* fix(blocks): log the dev-tunnel re-check failure instead of folding it into the refusal count
Delta round on the previous commit's fixes. The wrapper added there was right
about the verdict and wrong about observability.
A throw out of the dev-tunnel re-check used to reach `resolveRestApprovalVerdict`,
get logged through the throttled limiter with its error message, and answer
`lookup_failed`. Wrapping it turned that into a bare `catch` returning
`not_approved` — correct as a verdict, but it increments the SAME series this
whole change ships to be watched on. A sysRedis fault would have pushed every
population-E owner into `not_approved` fleet-wide with nothing logged anywhere,
and the predicate's own docblock reads that series as "the 4h window closing" —
so the incident would have read as the narrowing working. The one leg the change
added was the one leg it made unobservable.
It now logs, with its own message and its own throttle window. Deliberately a log
rather than a new verdict: a `tunnel_lookup_failed` would have to be mapped by both
callers, and the REST mapping for an unrecognised verdict is 503 — the exact
misattribution (a cache fault blamed on the replica read) the wrapper exists to
avoid. Separate windows because two failure modes sharing one would suppress each
other, and the one you did not see would be the one you needed.
The throttle logic is now written once and closed over per caller, rather than
open-coded twice.
Also from the same round:
- `parseSubjectUserId` and five self-scope gates still spelled 'anon' as a literal
while the new leaf claimed one spelling for the format. They use ANON_SUBJECT now,
so the claim is true rather than nearly true.
- `publisher-ban-revocation` took `subjectForUserId` through
`block-revocation.service`'s re-export — a module wholesale-mocked in a dozen
suites with a factory exporting only `BlockRevocation`, which is the shape the
leaf was extracted to avoid. It imports the leaf directly.
Refs clawgate #571.
* docs(blocks): correct four claims the last round's fix made false
Round 3 of the audit ladder. No behaviour change — every finding was a comment
that the code contradicts, which is the class this file keeps producing because
its docblocks carry the reasoning rather than just describing it.
1. The new tunnel-logger docblock said a dedicated verdict was avoided because
"the REST mapping for an unknown verdict is 503". It is not. The chain in
`withBlockScope` is not_approved -> 403, lookup_failed -> 503, and then an
`else` that asserts `satisfies 'not_found'`, logs "SERVING (observe-only)" and
falls through to the handler. REST's runtime default for an unrecognised
verdict is to SERVE, and it would log the request as a missing row it is not.
The bridge is the opposite: `satisfies never` then an unconditional FORBIDDEN.
So the paragraph told the next author REST fails closed on the exact branch
where it fails open. The real reasons — the two-caller mapping burden, forced
by the compile error at that `satisfies`, and REST's serve-by-default — are
now what it says. It also inverted the trade: a dedicated verdict would be
BETTER attribution than the log, since it would carry its own reason= label
instead of sharing not_approved. Recorded as such, so "add the verdict" stays
available as the deliberate change rather than looking already-rejected.
2. `LOOKUP_FAILURE_LOG_WINDOW_MS`'s docblock now sits above both loggers while
asserting "THE COUNT IS NOT THE ALERTING SIGNAL — the unthrottled
reason=lookup_failed series is". True of the replica-read logger, false of the
tunnel one, whose failures resolve to not_approved and have no dedicated label:
there the throttled log IS the only signal, so a suppressed line is lost
information rather than redundant prose. Same window, opposite relationship to
the metrics.
3. The predicate's "IT DOES NOT CATCH" heading is a blanket claim the previous
commit falsified 130 lines below it, and it names as an anti-pattern exactly
what the tunnel leg now does. Scoped to the ROW reads, with the reason the
exception is right there and wrong for them: an unreachable replica means "we
cannot establish whether this app may run", which is a different question per
caller; an unreachable tunnel cache means "no live tunnel", which is the same
fail-closed answer everywhere.
4. The bridge's "a read that THROWS propagates as the tRPC internal error" is no
longer true of the tunnel read — that caller now gets FORBIDDEN plus a warn
this path never emitted. Scoped, and flagged as the behaviour change it is.
Also: the new log test relied on being the first tunnel failure in the process
for its toHaveBeenCalledTimes(1). It resets the window instead, so it no longer
breaks based on where it sits in the file.
Refs clawgate #571.
* fix(blocks): give the dev-tunnel failure its own verdict — the log it had cannot be read here
Round 4 of the ladder, and it overturns round 2's fix rather than refining it.
Round 2 found the dev-tunnel re-check swallowing a throw into `not_approved`
with no signal, and answered it with a throttled `console.warn`. That answer is
inert on this deployment: `app-block-runtime.metrics.ts` states twice, and
designs around, the fact that application-container logs are NOT collected here
— "the `console.error` shape used elsewhere in the repo would be invisible to a
later investigator". So the fix swapped a silent swallow for an unreadable one,
and the paragraph arguing the log was the signal separating a cache incident
from the stale-token population was wrong about its own environment.
The leg now returns `tunnel_lookup_failed`. It refuses identically to
`not_approved` on both callers — same status, same message, deliberately, since
a bearer learning that the dev-tunnel cache is down would be an infrastructure
oracle — but it carries its own `reason=` label, so an operator can tell a
sysRedis fault from the population this change exists to create. That matters
because `not_approved` is the series the whole narrowing is watched on: folded in,
an incident reads as the fix working.
NOT reused `lookup_failed`, which was the tidier-looking option: that verdict
means the REPLICA read failed, answers 503, and is SERVED on the five routes
declaring `onApprovalLookupFailure: 'serve'`. Both would be wrong — a cache fault
blamed on the database, and a non-approved app served on some routes.
Four edit sites, all compiler-forced: the verdict union, both caller mappings,
and the metric's reason union. The cardinality guard went 3 -> 4 series; its
budget is a deliberate literal, so the new label is argued in place rather than
waved through, and the test now drives the new reason instead of only declaring
it.
Three prose corrections from the same round, each a claim the code denies:
- "the window is shared because the rate argument is identical" — it is not. The
replica logger's case is fleet-wide simultaneity; the tunnel logger is reachable
on one owner's one app. By this repo's own reasoning that shape needs no throttle
at all. Kept, with the real reason.
- "a dedicated verdict would carry its own label" was true only for REST: the
bridge records no verdict metric at all, and the label union is a third edit
site rather than derived.
- "`lookup_failed` -> 503" is route-dependent, the same shape of flat claim round 3
existed to remove, one verdict over.
Refs clawgate #571.
* docs(blocks): teach the verdict docs about the fourth reason, and stop claiming the bridge is covered
Round 5. No behaviour change. Adding `tunnel_lookup_failed` last round left six
places describing a three-verdict world, including the two an operator actually
reads, and repeated this change's own recurring mistake: writing a justification
one surface wider than the fix reaches.
THE ONE THAT MATTERS. The claim that a suppressed log now "loses prose and not
information" is true on REST only. `recordBlockRestApprovalVerdict` has a single
production call site, in `withBlockScope`; `assertAppBlockApproved` resolves the
same verdict and records nothing. So a tunnel-cache fault reached through the
BRIDGE emits no counter at all, and its only trace is the throttled warn — on a
deployment that does not collect container logs. That gap predates this work and
is equally true of `not_approved` (the bridge has never recorded a verdict), so
closing it is a bridge-metrics change rather than a guard one and is not taken
here. What is taken is saying so, in all three places that would otherwise let a
reader infer from the REST series that the bridge is covered — including the
explicit warning that a zero on `reason="tunnel_lookup_failed"` does not mean the
leg is healthy, given the bridge is the higher-rate surface (`pollWorkflow` is
timer-driven).
The operator-facing docs now know the label exists. The metric's `help` string
and its reader table enumerated three reasons and read as exhaustive, which is
the one place a description of `reason` reaches whoever is looking at the series
in Grafana — the whole point of the label was that someone can tell a sysRedis
fault from the stale-token population, and it was undocumented. Likewise the
verdict table in `block-scope.middleware`, which is the gate's own explanation of
the branch chain the last commit edited fourteen lines below it. That table now
carries the row that does NOT bend: `onApprovalLookupFailure` is scoped to
`lookup_failed` alone, so a route wanting lookup-failure tolerance does not get
it here — serving a known non-approved app because a cache was down is not
tolerating an unknown.
Count corrections: 3 -> 4 across the metric docblocks, the middleware table, the
emitter's own docblock and the metrics-test prose.
Two more claims the code denied:
- `resolveRestApprovalVerdict`'s docblock is the canonical mapping (the middleware
points at it rather than restating), and it still listed three verdicts AND
repeated the flat "`lookup_failed` (503) refuse" that the previous commit had
corrected 240 lines above. Fixed one copy, left the authoritative one.
- The `tunnelFailureLog` docblock still opened, present tense, with "converts a
throw into `not_approved`" while its own third paragraph said otherwise.
And two of my own from last round, which is the pattern:
- The new metrics-test comment claimed it proved a production caller emits the
reason. It cannot — the loop iterates the union, so a phantom nobody emits
satisfies it identically. The real pin is in the approved-gate suite, against
the real middleware; this one is the cardinality half and now says so.
- "That second shape does not need throttling at all" understated the tunnel
logger's case: a sysRedis incident hits every pod at once too, so it is the same
simultaneity over a smaller population, not a different shape.
Refs clawgate #571.
* docs(blocks): finish the fourth reason — the count said four where the list said three
Round 6. No behaviour change. Both findings are the previous commit's own half-done
edits, which is the pattern this ladder keeps producing.
The metrics test's reader block was bumped to "the four reasons" over an
enumeration that still listed three. Before that bump it was stale but coherent;
after it, a reader counting rows finds one reason undocumented and has to go
looking for which — and the missing row is the one carrying the new operational
fact, that `tunnel_lookup_failed` is 403 on every route because
`onApprovalLookupFailure` does not reach it. That block is the third reader table
in the same family as the metric `help` string and the middleware table; the other
two got the row and it did not.
Same edit, second instance: a test title went "the three reasons are SEPARATE
series" -> "the four", while its body still drove three. The fourth reason's
separateness was covered elsewhere, so nothing was unproven — but the title read
as the proof and was not, which is exactly the over-claim the same file corrects
two cases below. The case now drives what its title counts.
And the REST-only scoping from last round reached three sites but not the two
that most needed it:
- The `catch` comment inside the SHARED predicate said "the counter is the
signal", flat. That function is the one both callers use and the place a reader
is standing when they ask whether this is observable — and on the bridge there
is no counter.
- The `tunnelFailureLog` docblock closed with "no longer load-bearing", which
directly contradicted the paragraph ninety lines above it stating that a bridge
tunnel failure has this line as its only trace. Both cannot be true of one
logger. The asymmetry is now stated as the argument it is: for giving the bridge
a verdict counter, not for trusting the log.
Refs clawgate #571.
* docs(blocks): the comment welded two slips into one moment, and pointed at the wrong case
Round 7, and both findings are in the five-line comment round 6 added.
It said the reason was "left out when the reason was added — the title said four
while the body drove three". Checked against the branch: at the commit that added
the reason the title still said THREE, and the case was internally consistent —
under-covering the new reason, but claiming nothing it did not prove. The title
was bumped to four in the following docs pass, and that is where it became a
coverage claim wider than the test. Two slips, one commit apart, welded onto a
moment neither of them happened at. The commit message for that pass had the
history right; the in-file comment was a degraded restatement of it.
And "two cases below" is off by one — the case that corrects the same shape is
`emits AT MOST 4 series`, three below. Named rather than counted now, so it
cannot drift again when a case is inserted.
Refs clawgate #571.
This commit is contained in:
Zachary Lowden
@@ -34,7 +34,7 @@ import {
|
||||
* 🔴 The cardinality bound is the other load-bearing property, same as the siblings. This
|
||||
* counter fires once per non-ok REST request with nothing caching or rate-limiting it,
|
||||
* across every scraped pod, and prom-client retains every distinct label set in the Node
|
||||
* heap for the process lifetime. One label over a 3-value code-owned union = 3 series,
|
||||
* heap for the process lifetime. One label over a 4-value code-owned union = 4 series,
|
||||
* total, forever. Widening it is a code change that has to get past these tests.
|
||||
*/
|
||||
|
||||
@@ -72,17 +72,23 @@ describe('civitai_app_block_rest_approval_verdicts_total', () => {
|
||||
|
||||
/**
|
||||
* 🔴 THE SPLIT IS THE WHOLE VALUE OF THIS SIGNAL, and it is a sharper claim here than on
|
||||
* the sibling counters, because the three reasons do not even agree on whether the
|
||||
* the sibling counters, because the four reasons do not even agree on whether the
|
||||
* request was served:
|
||||
*
|
||||
* not_approved — REFUSED 403. The gate working; the only branch carrying its value.
|
||||
* not_found — SERVED. A healthy app; the false-positive channel.
|
||||
* lookup_failed — ROUTE-DEPENDENT: 503 on the routes that fail closed, SERVED on the
|
||||
* five that declare `onApprovalLookupFailure`. Infra, not policy.
|
||||
* tunnel_lookup_failed
|
||||
* — REFUSED 403 on EVERY route. Infra like `lookup_failed`, but a CACHE
|
||||
* fault rather than a replica one, and `onApprovalLookupFailure` does
|
||||
* NOT cover it: a known non-approved app must not be served anywhere
|
||||
* because a cache was down. Separate from `not_approved` so a sysRedis
|
||||
* incident is not counted as the dev-token narrowing working.
|
||||
*
|
||||
* `sum(rate(...))` across the label adds requests that were turned away to requests that
|
||||
* were served, so an operator who cannot split by `reason` has a number with no meaning.
|
||||
* Three separate series is what makes the split possible at all.
|
||||
* Four separate series is what makes the split possible at all.
|
||||
*
|
||||
* 🔴 BUT SPLITTING BY `reason` NO LONGER SETTLES WHAT HAPPENED, AND THIS DOCBLOCK USED TO
|
||||
* SAY IT DID (`lookup_failed — REFUSED 503`). Since the opt-out landed, `lookup_failed`
|
||||
@@ -117,15 +123,23 @@ describe('civitai_app_block_rest_approval_verdicts_total', () => {
|
||||
* attach point sits above the gate — and from `statusToRequestResult`. It is NOT exercised
|
||||
* by a test, because the middleware suites stub `res.on` as a no-op.)
|
||||
*/
|
||||
it('🔴 the three reasons are SEPARATE series — a served not_found never reads as a refusal', async () => {
|
||||
it('🔴 the four reasons are SEPARATE series — a served not_found never reads as a refusal', async () => {
|
||||
recordBlockRestApprovalVerdict('not_approved');
|
||||
recordBlockRestApprovalVerdict('not_found');
|
||||
recordBlockRestApprovalVerdict('not_found');
|
||||
recordBlockRestApprovalVerdict('lookup_failed');
|
||||
// The fourth reason is driven here too, so the title's count and the case's coverage
|
||||
// are the same number. Two separate slips, one commit apart: the body was not extended
|
||||
// when the reason was added (under-covered, but the title still said three, so the case
|
||||
// claimed nothing it did not prove), and the title was bumped to four in the following
|
||||
// docs pass — which is where it became a coverage claim wider than the test. The same
|
||||
// shape is corrected in the `emits AT MOST 4 series` case below.
|
||||
recordBlockRestApprovalVerdict('tunnel_lookup_failed');
|
||||
|
||||
expect(await readReason('not_approved')).toBe(1);
|
||||
expect(await readReason('not_found')).toBe(2);
|
||||
expect(await readReason('lookup_failed')).toBe(1);
|
||||
expect(await readReason('tunnel_lookup_failed')).toBe(1);
|
||||
});
|
||||
|
||||
it('🔴 DECLARES exactly one label, `reason` — the cardinality bound is structural', async () => {
|
||||
@@ -151,21 +165,32 @@ describe('civitai_app_block_rest_approval_verdicts_total', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('🔴 the reason union is EXACTLY these three values — 3 series is the whole budget', () => {
|
||||
it('🔴 the reason union is EXACTLY these four values — 4 series is the whole budget', () => {
|
||||
// Literal, not derived: this is the number an operator's cardinality budget is sized
|
||||
// against, and the union is simultaneously the metric label AND the non-`ok` half of
|
||||
// `AppBlockApprovalVerdict | 'lookup_failed'`, so a fourth verdict added on the service
|
||||
// `AppBlockApprovalVerdict | 'lookup_failed'`, so a new verdict added on the service
|
||||
// side has to come through here.
|
||||
//
|
||||
// ⚠️ WENT FROM THREE TO FOUR WITH clawgate #571, AND THE GUARD WORKING IS WHY.
|
||||
// `tunnel_lookup_failed` is the dev-tunnel re-check failing. It refuses exactly like
|
||||
// `not_approved` and could have shared its label for free — which is precisely what
|
||||
// this test exists to make someone argue for rather than default into. The argument
|
||||
// against sharing: that series is the one the dev-token narrowing ships to be watched
|
||||
// on, so a sysRedis fault folded into it reads as the narrowing working, and on this
|
||||
// deployment there is no log to fall back on (application-container logs are not
|
||||
// collected). One more series is the price of being able to tell an incident from the
|
||||
// population it would otherwise hide in.
|
||||
expect([...APP_BLOCK_REST_APPROVAL_VERDICT_REASONS]).toEqual([
|
||||
'not_approved',
|
||||
'not_found',
|
||||
'lookup_failed',
|
||||
'tunnel_lookup_failed',
|
||||
]);
|
||||
});
|
||||
|
||||
it('🔴 emits AT MOST 3 series no matter how many verdicts land', async () => {
|
||||
// The end-state assertion the label-name check implies: drive 300 verdicts across
|
||||
// every reason and the scrape still carries 3 lines for this metric.
|
||||
it('🔴 emits AT MOST 4 series no matter how many verdicts land', async () => {
|
||||
// The end-state assertion the label-name check implies: drive 400 verdicts across
|
||||
// every reason and the scrape still carries 4 lines for this metric.
|
||||
for (let i = 0; i < 100; i++) {
|
||||
for (const reason of APP_BLOCK_REST_APPROVAL_VERDICT_REASONS) {
|
||||
recordBlockRestApprovalVerdict(reason);
|
||||
@@ -175,8 +200,16 @@ describe('civitai_app_block_rest_approval_verdicts_total', () => {
|
||||
get(): Promise<{ values: Array<{ labels: Record<string, string> }> }>;
|
||||
};
|
||||
const { values } = await metric.get();
|
||||
expect(values).toHaveLength(3);
|
||||
expect(values).toHaveLength(4);
|
||||
expect(await readReason('not_found')).toBe(100);
|
||||
// ⚠️ THIS ASSERTS THE LOOP DROVE IT, NOT THAT A PRODUCTION CALLER DOES, and an earlier
|
||||
// comment here claimed the stronger thing. The loop iterates the union itself, so a
|
||||
// phantom reason no caller ever emits would satisfy this exactly as well. What pins a
|
||||
// real emitter is `block-scope.approved-gate.test.ts`'s
|
||||
// `expect(recordVerdictMock.mock.calls).toEqual([['tunnel_lookup_failed']])`, in a
|
||||
// different file and against the real middleware. Kept here only as the
|
||||
// cardinality-budget half: 100 increments on a 4th reason still yield one series.
|
||||
expect(await readReason('tunnel_lookup_failed')).toBe(100);
|
||||
});
|
||||
|
||||
it('is idempotent to register — a double module import does not throw', () => {
|
||||
@@ -191,7 +224,7 @@ describe('civitai_app_block_rest_approval_verdicts_total', () => {
|
||||
|
||||
/**
|
||||
* 🔴 THE ASSERTION AN ALERT RULE ACTUALLY DEPENDS ON — the exact scrape text, name and
|
||||
* label key and label VALUE, for all three reasons. Every other case in this file would
|
||||
* label key and label VALUE, for all four reasons. Every other case in this file would
|
||||
* still pass if the metric were renamed, because they all reach it through
|
||||
* `getSingleMetric(METRIC)` with the same constant; this one reads the rendered
|
||||
* exposition the scraper sees.
|
||||
|
||||
@@ -226,9 +226,9 @@ export type AppSpendCapRejectionReason = (typeof APP_SPEND_CAP_REJECTION_REASONS
|
||||
|
||||
/**
|
||||
* The NON-`ok` verdicts of `withBlockScope`'s approved-status gate. Kept as a code-owned
|
||||
* union (not a free string) so the `reason` label stays a bounded 3-series set.
|
||||
* union (not a free string) so the `reason` label stays a bounded 4-series set.
|
||||
*
|
||||
* 🔴 TWO OF THESE REFUSE AND ONE DOES NOT, which is why this is not called `…REFUSALS`:
|
||||
* 🔴 THREE OF THESE REFUSE AND ONE DOES NOT, which is why this is not called `…REFUSALS`:
|
||||
* `not_found` is counted and then SERVED. See the counter's own comment for the argument.
|
||||
*/
|
||||
/**
|
||||
@@ -301,6 +301,20 @@ export const APP_BLOCK_REST_APPROVAL_VERDICT_REASONS = [
|
||||
'not_approved',
|
||||
'not_found',
|
||||
'lookup_failed',
|
||||
/**
|
||||
* The dev-tunnel re-check could not be completed (clawgate #571). Refuses like
|
||||
* `not_approved`, counted separately so a cache incident is not indistinguishable from
|
||||
* the stale-dev-token population that verdict exists to create — which matters more
|
||||
* here than it would elsewhere, because application-container logs are not collected on
|
||||
* this deployment, so this label is the whole signal for that leg.
|
||||
*
|
||||
* ⚠️ THIS LIST IS A THIRD EDIT SITE, NOT DERIVED. It is a hand-maintained union
|
||||
* alongside `AppBlockApprovalVerdict` and the two callers' mappings; a new verdict needs
|
||||
* all four. The compiler does force it — `recordBlockRestApprovalVerdict(approval)` in
|
||||
* `block-scope.middleware` fails to type-check until the label exists — so this cannot
|
||||
* be forgotten silently, but it is easy to be surprised by.
|
||||
*/
|
||||
'tunnel_lookup_failed',
|
||||
] as const;
|
||||
export type AppBlockRestApprovalVerdictReason =
|
||||
(typeof APP_BLOCK_REST_APPROVAL_VERDICT_REASONS)[number];
|
||||
@@ -790,7 +804,7 @@ export function ensureRegisterAppBlockRuntimeMetrics(reg: Registry = client.regi
|
||||
// request whose verdict was NOT `ok`, by reason. `ok` and `dev_exempt` are not
|
||||
// counted: they are the steady state and would swamp the series.
|
||||
//
|
||||
// 🔴 READ THE `refused?` COLUMN BEFORE ALERTING ON THIS. Two of the three reasons
|
||||
// 🔴 READ THE `refused?` COLUMN BEFORE ALERTING ON THIS. Three of the four reasons
|
||||
// refuse and one deliberately does not, so `sum(rate(...))` across the label is a
|
||||
// number with no meaning — it adds requests that were turned away to requests that
|
||||
// were served. Always split by `reason`.
|
||||
@@ -811,11 +825,31 @@ export function ensureRegisterAppBlockRuntimeMetrics(reg: Registry = client.regi
|
||||
// needs fixing, NOT an authorization event and NOT an outage.
|
||||
// It is a separate series precisely so that it never has to be
|
||||
// inferred out of a combined "the gate refused something" number.
|
||||
// lookup_failed — REFUSED, 503. The replica read threw. Infra, not policy;
|
||||
// fail-closed, because a read we cannot complete leaves us
|
||||
// unable to establish that the app is allowed to run at all.
|
||||
// lookup_failed — REFUSED, 503 — on the routes that fail closed; the five
|
||||
// declaring `onApprovalLookupFailure: 'serve'` are SERVED, so the
|
||||
// outcome is ROUTE-DEPENDENT and this row cannot be read flat. The
|
||||
// replica read threw. Infra, not policy; fail-closed by default,
|
||||
// because a read we cannot complete leaves us unable to establish
|
||||
// that the app is allowed to run at all.
|
||||
// tunnel_lookup_failed
|
||||
// — REFUSED, 403, on every route. The dev-tunnel re-check could not
|
||||
// be completed (clawgate #571). Infra, not policy, like
|
||||
// `lookup_failed` — but a CACHE fault rather than a replica one, and
|
||||
// NOT route-tolerable: `onApprovalLookupFailure` is deliberately
|
||||
// scoped to `lookup_failed` alone, because a non-approved app must
|
||||
// not be served anywhere on the strength of a cache read failing.
|
||||
// It exists as its own reason so a sysRedis incident is not
|
||||
// indistinguishable from `not_approved`, which is the series the
|
||||
// dev-token narrowing is watched on — folded in, an incident reads
|
||||
// as that change working.
|
||||
// ⚠️ REST-ONLY, like this whole counter. The tRPC bridge resolves
|
||||
// the same verdict and records NOTHING, so a tunnel failure reached
|
||||
// through the bridge does not appear here at all. That gap is
|
||||
// pre-existing and equally true of `not_approved`; it is called out
|
||||
// because the bridge is the higher-rate surface (`pollWorkflow` is
|
||||
// timer-driven), so a zero here does not mean the leg is healthy.
|
||||
//
|
||||
// 🔴 ONE LABEL, `reason`, over a 3-value code-owned union → 3 series, TOTAL.
|
||||
// 🔴 ONE LABEL, `reason`, over a 4-value code-owned union → 4 series, TOTAL.
|
||||
// No `app_block_id`: this fires once per non-ok request with nothing caching or
|
||||
// rate-limiting it, and prom-client retains every distinct label set in the Node
|
||||
// heap forever across ~130 scraped pods. Attribution belongs in the caller's log
|
||||
@@ -824,7 +858,7 @@ export function ensureRegisterAppBlockRuntimeMetrics(reg: Registry = client.regi
|
||||
const restApprovalVerdictsTotal = getOrCreateCounter(
|
||||
reg,
|
||||
'civitai_app_block_rest_approval_verdicts_total',
|
||||
'Non-ok verdicts of the withBlockScope approved-status gate on App Block REST requests, by reason. NOT all refusals — split by reason before alerting: not_approved = the backing app_blocks row is not approved, REFUSED 403 (the gate enforcing a takedown); not_found = a signature-valid token resolved to no app_blocks row, SERVED (observe-only: a healthy app, counted so the false-positive rate is visible); lookup_failed = the replica read threw, and the outcome is ROUTE-DEPENDENT — 503 on the routes that fail closed, SERVED on the five that declare onApprovalLookupFailure. This counter carries ONLY `reason`, so it cannot itself tell refused from served on lookup_failed; which routes serve is the ledger LOOKUP_FAILURE_SERVE_RATIONALE in no-unguarded-block-rest-token.test.ts, and civitai_app_block_requests_total{endpoint,result} is the sibling series that carries endpoint',
|
||||
'Non-ok verdicts of the withBlockScope approved-status gate on App Block REST requests, by reason. NOT all refusals — split by reason before alerting: not_approved = the backing app_blocks row is not approved, REFUSED 403 (the gate enforcing a takedown); not_found = a signature-valid token resolved to no app_blocks row, SERVED (observe-only: a healthy app, counted so the false-positive rate is visible); lookup_failed = the replica read threw, and the outcome is ROUTE-DEPENDENT — 503 on the routes that fail closed, SERVED on the five that declare onApprovalLookupFailure; tunnel_lookup_failed = the dev-tunnel re-check could not be completed (a cache fault, not a replica one), REFUSED 403 on EVERY route because onApprovalLookupFailure does not cover it, kept separate from not_approved so a sysRedis incident is not counted as the dev-token narrowing working. This counter carries ONLY `reason`, so it cannot itself tell refused from served on lookup_failed; which routes serve is the ledger LOOKUP_FAILURE_SERVE_RATIONALE in no-unguarded-block-rest-token.test.ts, and civitai_app_block_requests_total{endpoint,result} is the sibling series that carries endpoint',
|
||||
['reason']
|
||||
);
|
||||
|
||||
@@ -1302,7 +1336,7 @@ export function recordBlockRevocationRefusal(
|
||||
*
|
||||
* 🔴 TOTAL, like every emitter in this module, and here the reason is sharper than
|
||||
* usual: the thing it instruments is an authorization gate on the block REST surface,
|
||||
* and two of its three reasons are decided refusals. If a metrics error propagated, a
|
||||
* and three of its four reasons are decided refusals. If a metrics error propagated, a
|
||||
* verdict the gate had already settled would leave as an uncaught 500 instead of the
|
||||
* 403/503 it chose — or, on `not_found`, would turn a request the gate decided to SERVE
|
||||
* into a 500. Either way the observability would change the response it exists to
|
||||
|
||||
@@ -34,6 +34,24 @@ vi.mock('~/server/metrics/app-block-runtime.metrics', async (importOriginal) =>
|
||||
...((await importOriginal()) as Record<string, unknown>),
|
||||
recordBlockRestApprovalVerdict: recordVerdictMock,
|
||||
}));
|
||||
/**
|
||||
* The dev-tunnel lookup the predicate re-derives for a dev token on a REAL, NOT-approved
|
||||
* row. Stubbed rather than run for real because the real one is two sysRedis GETs against
|
||||
* the k8s-backed tunnel control plane — the SEAM is what this suite is about. Note the
|
||||
* predicate reaches it through `await import(...)`; `vi.mock` intercepts that exactly as
|
||||
* it does a static import, which is itself worth pinning: the dynamic form is there to
|
||||
* keep the k8s client off the REST middleware's load graph, not to dodge the seam.
|
||||
*/
|
||||
const { tunnelMock } = vi.hoisted(() => ({ tunnelMock: vi.fn() }));
|
||||
// `importOriginal` spread rather than a hand-listed factory: this module exports a dozen
|
||||
// other things (`touchDevTunnelActivity`, the session types, the reaper), and a factory
|
||||
// naming only the one function under test fails to LOAD the moment any module in the graph
|
||||
// imports a second export — with a green typecheck, a green lint and an error far from the
|
||||
// change. Replace the one export; keep the rest real.
|
||||
vi.mock('~/server/services/blocks/dev-tunnel.service', async (importOriginal) => ({
|
||||
...((await importOriginal()) as Record<string, unknown>),
|
||||
getActiveDevTunnel: (...a: unknown[]) => tunnelMock(...a),
|
||||
}));
|
||||
|
||||
import { dbMock } from '~/__tests__/mocks';
|
||||
import { withBlockScope } from '../block-scope.middleware';
|
||||
@@ -51,14 +69,25 @@ import { BlockTokenService } from '~/server/services/block-token.service';
|
||||
* only on the return value.
|
||||
*/
|
||||
const findUniqueMock = dbMock.dbRead.appBlock.findUnique;
|
||||
/**
|
||||
* The OWNER lookup, resolved in the dev + real-row + NOT-approved branch only. Separate
|
||||
* from the row read above on purpose — see the `select` assertion below for why it is not
|
||||
* a nested relation select — so it gets its own handle and its own call-count assertions.
|
||||
*/
|
||||
const oauthMock = dbMock.dbRead.oauthClient.findUnique;
|
||||
|
||||
const APP_ID = 'app_gate';
|
||||
const BLOCK_ID = 'blk_gate';
|
||||
const SCOPE = 'user:read:self';
|
||||
|
||||
async function mint(opts: { dev?: boolean } = {}): Promise<string> {
|
||||
/** The app owner in every fixture below, unless a test deliberately diverges from it. */
|
||||
const OWNER_ID = 42;
|
||||
|
||||
async function mint(
|
||||
opts: { dev?: boolean; reviewRunForReal?: boolean; userId?: number } = {}
|
||||
): Promise<string> {
|
||||
const { token } = await BlockTokenService.sign({
|
||||
userId: 42,
|
||||
userId: opts.userId ?? OWNER_ID,
|
||||
blockId: BLOCK_ID,
|
||||
appId: APP_ID,
|
||||
appBlockId: 'apb_gate',
|
||||
@@ -66,6 +95,7 @@ async function mint(opts: { dev?: boolean } = {}): Promise<string> {
|
||||
scopes: [SCOPE],
|
||||
ctx: {},
|
||||
...(opts.dev ? { dev: true } : {}),
|
||||
...(opts.reviewRunForReal ? { reviewRunForReal: true } : {}),
|
||||
} as Parameters<typeof BlockTokenService.sign>[0]);
|
||||
return token;
|
||||
}
|
||||
@@ -133,8 +163,18 @@ beforeEach(() => {
|
||||
// from, so the previous test's `mockResolvedValue`/`mockRejectedValue` would otherwise
|
||||
// survive into the next one.
|
||||
findUniqueMock.mockReset();
|
||||
// Default world for the owner lookup: the app IS owned by the fixture subject, so a test
|
||||
// that does not care about ownership is not refused by it. The tests that ARE about
|
||||
// ownership override it.
|
||||
oauthMock.mockReset();
|
||||
oauthMock.mockResolvedValue({ userId: OWNER_ID });
|
||||
isFliptMock.mockImplementation(async (flag: string) => flag === 'app-blocks-runtime-enabled');
|
||||
isRevokedMock.mockImplementation(async () => false);
|
||||
// Default world: NO active dev tunnel. The exempting condition must be opted INTO by
|
||||
// the tests that are about it, so a test that forgets fails closed rather than
|
||||
// inheriting an exemption from a previous case.
|
||||
tunnelMock.mockReset();
|
||||
tunnelMock.mockResolvedValue(null);
|
||||
});
|
||||
|
||||
describe('resolveRestApprovalVerdict — the predicate on its own', () => {
|
||||
@@ -182,35 +222,252 @@ describe('resolveRestApprovalVerdict — the predicate on its own', () => {
|
||||
// a second, looser `where` key alongside it.
|
||||
expect(findUniqueMock.mock.calls[0][0]).toEqual({
|
||||
where: { appId_blockId: { appId: APP_ID, blockId: BLOCK_ID } },
|
||||
// 🔴 STILL `{ status: true }`, AND THE NARROWNESS IS LOAD-BEARING. The owner column
|
||||
// this gate now consults is NOT selected here: without `relationJoins` a nested
|
||||
// relation select is a second round trip, not a wider row, so folding it in would
|
||||
// bill every bridge call and every REST request — `pollWorkflow` included — for a
|
||||
// column only the dev + non-approved branch reads. It is resolved in that branch
|
||||
// instead. A reviewer widening this select is the regression this line catches.
|
||||
select: { status: true },
|
||||
});
|
||||
});
|
||||
|
||||
it('a dev token is exempt AND does not even read the DB', async () => {
|
||||
/**
|
||||
* 🔴 THE DEV EXEMPTION IS NO LONGER A SHORT-CIRCUIT, AND THAT IS THE FIX (clawgate
|
||||
* #571). This test asserted the opposite until now — `dev: true` returned `dev_exempt`
|
||||
* against a `suspended` row WITHOUT reading it, so the verdict was independent of the
|
||||
* app's status for the whole 4h dev lifetime. The read is what makes the verdict
|
||||
* status-dependent, so "a dev token skips the DB read" was a description of the hole.
|
||||
*/
|
||||
it('a dev token no longer skips the read — the row is what decides', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, dev: true })).toBe('dev_exempt');
|
||||
// The second half is what pins the exemption as a SHORT-CIRCUIT rather than a
|
||||
// post-hoc override — and it is also the cost claim in the docblock (a dev token
|
||||
// skips the replica read).
|
||||
oauthMock.mockResolvedValue({ userId: 42 });
|
||||
await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true });
|
||||
expect(findUniqueMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('a dev token on an APPROVED row → ok, without needing any exemption', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'approved' });
|
||||
oauthMock.mockResolvedValue({ userId: 42 });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true })).toBe('ok');
|
||||
// Not `dev_exempt`: an approved app is approved. Pinning the verdict NAME here is
|
||||
// what stops a future "just exempt dev again" from passing this file — it would
|
||||
// still serve, but the counter would stop being able to say why.
|
||||
expect(tunnelMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* POPULATION B / C / D / F — the synthetic-id mints (`pubreq_…`, `page_local_…`,
|
||||
* `ephemeral-…`). They sign an `appId` that is not an `OauthClient.id`, so the unique
|
||||
* resolves to nothing. They have no row to be approved and must keep running; this is
|
||||
* the half of the old blanket exemption that was load-bearing.
|
||||
*/
|
||||
it('a dev token with NO backing row stays exempt — nothing to be approved', async () => {
|
||||
findUniqueMock.mockResolvedValue(null);
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true })).toBe(
|
||||
'dev_exempt'
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 POPULATION F′ — the moderator run-for-real review sandbox. Answered from the signed
|
||||
* claim BEFORE the read, because a review token names a `pubreq_` id that resolves to
|
||||
* no row. The `not.toHaveBeenCalled()` half is the one that matters: it pins this as a
|
||||
* claim-driven decision rather than an accident of the row being missing, so the
|
||||
* sandbox keeps working even if a `pubreq_` id ever did resolve to something.
|
||||
*/
|
||||
it('a run-for-real REVIEW token is exempt from the claim alone, before any read', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 9999 });
|
||||
expect(
|
||||
await resolveRestApprovalVerdict({ ...claims, sub: 'user:7', dev: true, reviewRunForReal: true })
|
||||
).toBe('dev_exempt');
|
||||
expect(findUniqueMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 POPULATION E — the owner dev-tunnel mint. The case the exemption exists FOR: an
|
||||
* app that is deliberately suspended/pending/deprecated stays runnable by its OWNER
|
||||
* inside the owner's OWN active dev tunnel, so they can diagnose it back into review.
|
||||
*/
|
||||
it('POPULATION E: owner + ACTIVE dev tunnel on a suspended app → exempt', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 42 });
|
||||
tunnelMock.mockResolvedValue({ sessionId: 'sess_1', userId: 42, blockId: BLOCK_ID });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true })).toBe(
|
||||
'dev_exempt'
|
||||
);
|
||||
// Keyed on the OWNER id resolved from the row and the token's own blockId — never on
|
||||
// anything a caller could choose.
|
||||
expect(tunnelMock).toHaveBeenCalledWith(42, BLOCK_ID);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 POPULATION A — THE DEFECT. A `dev:live` token minted through
|
||||
* `/api/v1/blocks/dev-token`'s approved mode, whose mint REQUIRED `status: 'approved'`.
|
||||
* It is owner-held and claim-identical to population E; the only thing separating them
|
||||
* is the active dev tunnel E's mint requires and A's does not. Before clawgate #571
|
||||
* this returned `dev_exempt` and kept driving the bridge for up to 4h after a
|
||||
* moderator suspension.
|
||||
*/
|
||||
it('POPULATION A: owner but NO active dev tunnel on a suspended app → not_approved', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 42 });
|
||||
tunnelMock.mockResolvedValue(null);
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true })).toBe(
|
||||
'not_approved'
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* Ownership can TRANSFER. A dev token outstanding against an app that has changed
|
||||
* hands must not stay exempt on the strength of the previous owner's subject — and the
|
||||
* tunnel lookup must not even be reached, since it is keyed on the CURRENT owner.
|
||||
*/
|
||||
it('a dev token whose subject is NOT the current owner → not_approved, no tunnel read', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 99 });
|
||||
tunnelMock.mockResolvedValue({ sessionId: 'sess_1', userId: 42, blockId: BLOCK_ID });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true })).toBe(
|
||||
'not_approved'
|
||||
);
|
||||
expect(tunnelMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* FAILS CLOSED when the owner cannot be resolved at all.
|
||||
*
|
||||
* ⚠️ INVARIANT GUARD, NOT REGRESSION COVERAGE, and labelled as one: `AppBlock.app` is a
|
||||
* required relation and `OauthClient.userId` a non-nullable `Int`, so a real row whose
|
||||
* owner lookup misses means the app was deleted between the two reads. Unreachable in
|
||||
* practice; pinned because the alternative to refusing is comparing the subject against
|
||||
* the literal string `user:undefined`, which is the shape that quietly becomes an
|
||||
* exemption if someone later "simplifies" the null check away.
|
||||
*/
|
||||
it('fails closed when the owner row cannot be resolved', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue(null);
|
||||
tunnelMock.mockResolvedValue({ sessionId: 'sess_1' });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:42', dev: true })).toBe(
|
||||
'not_approved'
|
||||
);
|
||||
expect(tunnelMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('fails closed for an anon-subject dev token — `anon` matches no owner', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 42 });
|
||||
tunnelMock.mockResolvedValue({ sessionId: 'sess_1' });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'anon', dev: true })).toBe(
|
||||
'not_approved'
|
||||
);
|
||||
expect(tunnelMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE EXEMPTION IS `=== true`, NOT TRUTHINESS, and this is the half a reviewer
|
||||
* would not think to write. `verifyBlockToken` already rejects a non-boolean `dev`
|
||||
* outright, so a string can't reach here through the real path — but the predicate is
|
||||
* exported and reachable from the shared-storage resolvers' neighbourhood, and a
|
||||
* truthy check would turn any future non-boolean into a silent exemption.
|
||||
*
|
||||
* 🔴 THE FIXTURE CLEARS EVERY OTHER REASON TO REFUSE, AND THAT IS THE WHOLE TEST.
|
||||
* It used to be a bare `{ status: 'suspended' }` with no `sub` and no owner. When the
|
||||
* `dev` check moved behind the row read (clawgate #571), that fixture started landing
|
||||
* on the OWNERSHIP guard's boundary instead — `block.app` was undefined, so
|
||||
* `ownerUserId == null` refused first and the mutant `!claims.dev` SURVIVED the whole
|
||||
* suite while this test stayed green and kept claiming to pin `=== true`. The guard was
|
||||
* never re-run; only the fixture had stopped reaching it. So: owner row, matching
|
||||
* subject, live tunnel — with `dev === true` every one of these would be `dev_exempt`,
|
||||
* which makes `not_approved` attributable to the `dev` comparison and nothing else.
|
||||
*/
|
||||
it.each([undefined, false, 0, '', 'true', 1, {}])(
|
||||
'dev=%p is NOT exempt — the check is `=== true`',
|
||||
'dev=%p is NOT exempt — the check is `=== true`, with every other refusal cleared',
|
||||
async (dev) => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: OWNER_ID });
|
||||
tunnelMock.mockResolvedValue({ sessionId: 'sess_1' });
|
||||
expect(
|
||||
await resolveRestApprovalVerdict({
|
||||
...claims,
|
||||
sub: `user:${OWNER_ID}`,
|
||||
dev,
|
||||
} as typeof claims)
|
||||
).toBe('not_approved');
|
||||
}
|
||||
);
|
||||
|
||||
/**
|
||||
* The SAME truthiness boundary on the no-row branch, which is a second `=== true` and
|
||||
* was uncovered: `claims.dev === true ? 'dev_exempt' : 'not_found'`. Every other no-row
|
||||
* case in this file uses either a real `dev: true` or no `dev` at all, so a truthy
|
||||
* mutant there survived them all.
|
||||
*/
|
||||
it.each([undefined, false, 0, '', 'true', 1, {}])(
|
||||
'dev=%p with NO row is not_found, not exempt — the check is `=== true` there too',
|
||||
async (dev) => {
|
||||
findUniqueMock.mockResolvedValue(null);
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, dev } as typeof claims)).toBe(
|
||||
'not_approved'
|
||||
'not_found'
|
||||
);
|
||||
}
|
||||
);
|
||||
|
||||
/**
|
||||
* And the THIRD `=== true`, on the widest bypass in the function — answered before any
|
||||
* read, for a subject that need not own the app. `verifyBlockToken` type-checks this
|
||||
* claim, so a non-boolean cannot arrive through the real path; the table exists for the
|
||||
* same reason the `dev` one does. The fixture is a suspended row owned by someone else,
|
||||
* so a truthy mutant would show up as `dev_exempt` rather than the refusal asserted.
|
||||
*/
|
||||
it.each([undefined, false, 0, '', 'true', 1, {}])(
|
||||
'reviewRunForReal=%p is NOT exempt — the check is `=== true`',
|
||||
async (reviewRunForReal) => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 99 });
|
||||
expect(
|
||||
await resolveRestApprovalVerdict({
|
||||
...claims,
|
||||
sub: `user:${OWNER_ID}`,
|
||||
dev: true,
|
||||
reviewRunForReal,
|
||||
} as typeof claims)
|
||||
).toBe('not_approved');
|
||||
}
|
||||
);
|
||||
|
||||
/**
|
||||
* 🔴 THE REVIEW BYPASS REQUIRES `dev` TOO, and that pairing is not decoration. Every
|
||||
* mint that stamps `reviewRunForReal` also stamps `dev`, so this is unreachable through
|
||||
* the real path today — but `BlockTokenService.sign` accepts the field independently,
|
||||
* and a bypass keyed on ONE signed boolean without narrowing it is the exact defect this
|
||||
* change exists to fix. Taking `reviewRunForReal` alone would have made the new
|
||||
* exemption WIDER than the blanket one it replaced.
|
||||
*/
|
||||
it('reviewRunForReal WITHOUT dev is not exempt — the pairing is the narrowing', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: 99 });
|
||||
expect(
|
||||
await resolveRestApprovalVerdict({
|
||||
...claims,
|
||||
sub: `user:${OWNER_ID}`,
|
||||
reviewRunForReal: true,
|
||||
} as typeof claims)
|
||||
).toBe('not_approved');
|
||||
});
|
||||
|
||||
/**
|
||||
* An APPROVED row short-circuits before ownership is ever consulted, so a dev token held
|
||||
* by a NON-owner on an approved app still serves. Without this, the suite could not tell
|
||||
* "approved wins" from "ownership also applies to approved rows" — every other
|
||||
* approved+dev fixture makes the subject the owner.
|
||||
*/
|
||||
it('an approved row serves a dev token held by a NON-owner — approved wins first', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'approved' });
|
||||
expect(await resolveRestApprovalVerdict({ ...claims, sub: 'user:7', dev: true })).toBe('ok');
|
||||
expect(oauthMock).not.toHaveBeenCalled();
|
||||
expect(tunnelMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('withBlockScope — the gate on the real request path', () => {
|
||||
@@ -446,15 +703,107 @@ describe('withBlockScope — the gate on the real request path', () => {
|
||||
|
||||
/**
|
||||
* 🔴 THE MODERATOR REVIEW SANDBOX, which is the reason the exemption exists and the
|
||||
* thing a gate without one would break. A run-for-real review token is `dev: true`, and
|
||||
* review is the ONE surface that must work on a non-approved app.
|
||||
* thing a too-tight narrowing would break — QUIETLY, since review would simply stop
|
||||
* being possible for pending apps and nothing would say so. Criterion 5 of clawgate
|
||||
* #571: this is the guard against the bad direction and is pinned on the REAL request
|
||||
* path, not only at the predicate.
|
||||
*
|
||||
* Note the fixture: a moderator (`user:7`) who is NOT the app's owner, on a suspended
|
||||
* row, with NO active dev tunnel — i.e. every other exempting condition absent, so the
|
||||
* `reviewRunForReal` claim is provably the only reason this serves.
|
||||
*/
|
||||
it('a DEV token on a SUSPENDED app still runs — the review sandbox is not broken', async () => {
|
||||
it('POSITIVE: a run-for-real REVIEW token still runs on a SUSPENDED app', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: OWNER_ID });
|
||||
tunnelMock.mockResolvedValue(null);
|
||||
const { handler, res } = await drive(await mint({ dev: true, reviewRunForReal: true, userId: 7 }));
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(res.statusCode).toBe(200);
|
||||
// Answered from the signed claim, before the row is even read.
|
||||
expect(findUniqueMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* POPULATION E on the real request path — the owner running their own suspended app
|
||||
* inside their own active dev tunnel. The other half of "do not break the thing the
|
||||
* exemption exists for".
|
||||
*/
|
||||
it('POSITIVE: the OWNER with an ACTIVE dev tunnel still runs on a SUSPENDED app', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: OWNER_ID });
|
||||
tunnelMock.mockResolvedValue({ sessionId: 'sess_1', userId: OWNER_ID, blockId: BLOCK_ID });
|
||||
const { handler, res } = await drive(await mint({ dev: true }));
|
||||
expect(handler).toHaveBeenCalledTimes(1);
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(findUniqueMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE DEFECT, on the real request path. Population A: a `dev:live` token minted while
|
||||
* the app was approved, still inside its 4h lifetime, after a moderator suspension —
|
||||
* same owner, same claims as the test directly above, and the ONLY difference is the
|
||||
* absent dev tunnel. Before clawgate #571 this served a 200.
|
||||
*/
|
||||
it('NEGATIVE: a stale dev token with NO tunnel is 403d on a SUSPENDED app', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: OWNER_ID });
|
||||
tunnelMock.mockResolvedValue(null);
|
||||
const { handler, res } = await drive(await mint({ dev: true }));
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
expect(res.statusCode).toBe(403);
|
||||
expect(res.body).toEqual({ error: 'app block is not approved' });
|
||||
// 🔴 AND IT IS COUNTED. This change creates a brand-new population of `not_approved`
|
||||
// refusals — stale dev tokens — and the counter is the only way an operator sees the
|
||||
// 4h window actually closing. Every other test in the verdict-counter block uses a
|
||||
// NON-dev token, so without this line the new population is invisible to the series
|
||||
// the docblock leans on when it says "ship it where it can be watched".
|
||||
expect(recordVerdictMock.mock.calls).toEqual([['not_approved']]);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 A THROW OUT OF THE TUNNEL LOOKUP MUST REFUSE, NOT 503. `getActiveDevTunnel`
|
||||
* swallows a rejected read, a deadline and a parse error — but it attaches its
|
||||
* `.catch()` to the RESULT of `sysRedis.get(...)`, so a SYNCHRONOUS throw from the
|
||||
* client escapes it, as can the `await import(...)` itself. Unwrapped, that escape is
|
||||
* caught one level up as `lookup_failed` and answered 503 — attributing a cache fault to
|
||||
* the replica read and pointing an incident at the wrong subsystem. The predicate wraps
|
||||
* it for exactly this, and this is the test that would notice the wrapper being removed.
|
||||
*/
|
||||
it('a THROW from the tunnel lookup refuses (403), it does not become a 503', async () => {
|
||||
findUniqueMock.mockResolvedValue({ status: 'suspended' });
|
||||
oauthMock.mockResolvedValue({ userId: OWNER_ID });
|
||||
tunnelMock.mockImplementation(() => {
|
||||
throw new Error('redis client exploded synchronously');
|
||||
});
|
||||
// 🔴 RESET THE WINDOW, DO NOT RELY ON BEING FIRST. `toHaveBeenCalledTimes(1)` holds
|
||||
// today only because this is the only test in the file that reaches the tunnel logger,
|
||||
// so it is necessarily that logger's first occurrence. Add a second tunnel-throw case
|
||||
// ABOVE this one and the assertion silently lands inside the 60s window and sees zero
|
||||
// calls — a test that breaks because of where it sits in the file. One line removes
|
||||
// the ordering dependency entirely.
|
||||
__resetApprovalLookupFailureLogThrottleForTests();
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||
try {
|
||||
const { handler, res } = await drive(await mint({ dev: true }));
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
expect(res.statusCode).toBe(403);
|
||||
expect(res.body).toEqual({ error: 'app block is not approved' });
|
||||
// 🔴 ITS OWN LABEL, AND THIS IS THE ASSERTION THAT MATTERS. Folded into
|
||||
// `not_approved` a sysRedis fault would land on the exact series this change ships
|
||||
// to be watched on and read as the narrowing working. Two earlier rounds answered
|
||||
// that with a throttled `console.warn` instead — which cannot work on this
|
||||
// deployment, because application-container logs are not collected
|
||||
// (`app-block-runtime.metrics.ts` says so twice and designs around it). The counter
|
||||
// is the signal; asserting the label here is what stops it being folded back.
|
||||
expect(recordVerdictMock.mock.calls).toEqual([['tunnel_lookup_failed']]);
|
||||
// The log is kept for environments that DO collect container logs, and is asserted
|
||||
// distinctly from the replica-read message ("approved-status lookup failed") so the
|
||||
// two failure modes stay separable there too — but it is no longer the signal.
|
||||
expect(warn).toHaveBeenCalledTimes(1);
|
||||
expect(warn.mock.calls[0][0]).toContain('[block-scope] dev-tunnel re-check failed');
|
||||
expect(warn.mock.calls[0][0]).toContain('redis client exploded synchronously');
|
||||
} finally {
|
||||
warn.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
DEV_TOKEN_LIFETIME_SECONDS,
|
||||
getBlockTokenVerificationKeysByKid,
|
||||
} from '~/server/services/block-token.service';
|
||||
import { ANON_SUBJECT, isValidSubject, USER_SUB_RE } from '~/server/services/block-token-subject';
|
||||
import { isKnownBlockScope } from '~/shared/constants/block-scope.constants';
|
||||
import {
|
||||
isBlockActionDetail,
|
||||
@@ -68,9 +69,18 @@ export interface BlockTokenClaims {
|
||||
/** Advisory: the color domain the token was minted on (`green`|`blue`|`red`). */
|
||||
domain?: string;
|
||||
/**
|
||||
* DEV-TOKEN marker — present (true) ONLY on tokens minted by the mod-gated
|
||||
* DEV-TOKEN marker. ⚠️ THIS USED TO SAY "ONLY on tokens minted by the mod-gated
|
||||
* dev-token endpoint (`/api/v1/blocks/dev-token`) for the `dev:live` localhost
|
||||
* harness. It selects the per-token-type max-age cap in `verifyBlockToken`
|
||||
* harness", and that is wrong in the direction that matters: the claim is stamped
|
||||
* unconditionally by `signDevScopedPageToken`, which is reached by SIX mint paths
|
||||
* across `/api/v1/blocks/dev-token` (approved / pending / local-manifest),
|
||||
* `/api/v1/block-tokens` (ephemeral tunnel / owner-non-approved tunnel) and the tRPC
|
||||
* review-sandbox mint. So `dev === true` identifies a LIFETIME class, not a caller and
|
||||
* not a capability — reading it as "the dev:live harness" is how a guard came to exempt
|
||||
* all six from the approved-status check (clawgate #571). Anything deciding
|
||||
* AUTHORIZATION on this claim must narrow it further; see
|
||||
* `resolveAppBlockApprovalVerdict` for the population table.
|
||||
* It selects the per-token-type max-age cap in `verifyBlockToken`
|
||||
* (4h for dev, 15min for every other token). The claim is only trustworthy
|
||||
* BECAUSE the signature (RS256, our kid) is verified before it's read — a
|
||||
* forged `dev:true` can't pass the signature gate. The claim is optional and
|
||||
@@ -618,15 +628,14 @@ export async function verifyBlockToken(token: string): Promise<BlockTokenClaims
|
||||
return null;
|
||||
}
|
||||
|
||||
// M4: cap digit length to keep `user:<unbounded digits>` from sliding past
|
||||
// Number.MAX_SAFE_INTEGER and producing a silent mis-match against ctx.modelId.
|
||||
// 12 digits is well above any realistic civitai userId (~10 digits = 9.9B).
|
||||
const USER_SUB_RE = /^user:[1-9][0-9]{0,11}$/;
|
||||
|
||||
/** True iff `sub` is one of the two valid shapes: `anon` or `user:<positive int>`. */
|
||||
export function isValidSubject(sub: string): boolean {
|
||||
return sub === 'anon' || USER_SUB_RE.test(sub);
|
||||
}
|
||||
// ⚠️ `USER_SUB_RE` and `isValidSubject` MOVED to `~/server/services/block-token-subject`
|
||||
// — a zero-import leaf shared with the MINT (`block-token.service`), the revocation
|
||||
// writer and the approval guard, so the format has one spelling instead of one per
|
||||
// consumer. Re-exported here because this module is where every existing caller imports
|
||||
// it from. The regex's own rationale travels with it: capping the digit length keeps
|
||||
// `user:<unbounded digits>` from sliding past Number.MAX_SAFE_INTEGER and producing a
|
||||
// silent mis-match against ctx.modelId.
|
||||
export { isValidSubject };
|
||||
|
||||
/**
|
||||
* Extracts the userId from a verified `sub` claim. Use AFTER isValidSubject.
|
||||
@@ -635,7 +644,7 @@ export function isValidSubject(sub: string): boolean {
|
||||
* via isValidSubject won't see throws in practice.
|
||||
*/
|
||||
export function parseSubjectUserId(sub: string): number | null {
|
||||
if (sub === 'anon') return null;
|
||||
if (sub === ANON_SUBJECT) return null;
|
||||
if (!USER_SUB_RE.test(sub)) {
|
||||
throw forbidden('malformed sub claim');
|
||||
}
|
||||
@@ -705,7 +714,7 @@ export function enforceContextBinding(claims: BlockTokenClaims, req: NextApiRequ
|
||||
// Every :self scope requires an authenticated subject — there's no
|
||||
// anonymous "self" to read/tip. user:read:self joined this set
|
||||
// when /api/v1/blocks/me switched off buzz:read:self (audit I3).
|
||||
if (claims.sub === 'anon') {
|
||||
if (claims.sub === ANON_SUBJECT) {
|
||||
throw forbidden(`${scope} requires authenticated subject`);
|
||||
}
|
||||
break;
|
||||
@@ -725,7 +734,7 @@ export function enforceContextBinding(claims: BlockTokenClaims, req: NextApiRequ
|
||||
// actual KV read/write happens; this case exists so adding these
|
||||
// scopes to BLOCK_SCOPE_TO_OAUTH_BIT does NOT silently reintroduce the
|
||||
// fail-open the comment below warns about (audit fix 3 / L-M6).
|
||||
if (claims.sub === 'anon') {
|
||||
if (claims.sub === ANON_SUBJECT) {
|
||||
throw forbidden(`${scope} requires authenticated subject`);
|
||||
}
|
||||
break;
|
||||
@@ -743,7 +752,7 @@ export function enforceContextBinding(claims: BlockTokenClaims, req: NextApiRequ
|
||||
// anon subject here so wiring this scope can't silently fail open (mirrors
|
||||
// the apps:storage:write case). The trust gate itself is enforced in
|
||||
// `resolveSharedContext`.
|
||||
if (claims.sub === 'anon') {
|
||||
if (claims.sub === ANON_SUBJECT) {
|
||||
throw forbidden(`${scope} requires authenticated subject`);
|
||||
}
|
||||
break;
|
||||
@@ -760,7 +769,7 @@ export function enforceContextBinding(claims: BlockTokenClaims, req: NextApiRequ
|
||||
// the read:private scope) + the maturity clamp; the follow write is
|
||||
// self-bound to this subject. No request-shape binding is added here —
|
||||
// presence of the scope + a non-anon subject is the middleware check.
|
||||
if (claims.sub === 'anon') {
|
||||
if (claims.sub === ANON_SUBJECT) {
|
||||
throw forbidden(`${scope} requires authenticated subject`);
|
||||
}
|
||||
break;
|
||||
@@ -779,7 +788,7 @@ export function enforceContextBinding(claims: BlockTokenClaims, req: NextApiRequ
|
||||
// omitting it bricks the whole app and reads as a bug in an unrelated
|
||||
// endpoint. It must land in the same commit as the
|
||||
// BLOCK_SCOPE_TO_OAUTH_BIT entry.
|
||||
if (claims.sub === 'anon') {
|
||||
if (claims.sub === ANON_SUBJECT) {
|
||||
throw forbidden(`${scope} requires authenticated subject`);
|
||||
}
|
||||
break;
|
||||
@@ -968,7 +977,7 @@ export function withBlockScope(handler: NextApiHandler, opts: WithBlockScopeOpts
|
||||
// on `resolveRestApprovalVerdict` in
|
||||
// `~/server/services/blocks/block-approval.service` — one docblock, not two.
|
||||
//
|
||||
// 🔴 WHICH VERDICTS REFUSE IS NOT UNIFORM ACROSS THE THREE, AND FOR ONE OF THEM IT IS
|
||||
// 🔴 WHICH VERDICTS REFUSE IS NOT UNIFORM ACROSS THE FOUR, AND FOR ONE OF THEM IT IS
|
||||
// NOT UNIFORM ACROSS ROUTES EITHER.
|
||||
//
|
||||
// `not_approved` — ALWAYS 403, on every route. This branch carries the whole of the
|
||||
@@ -985,8 +994,19 @@ export function withBlockScope(handler: NextApiHandler, opts: WithBlockScopeOpts
|
||||
// NO row is a HEALTHY app — a row deleted or re-keyed mid-session,
|
||||
// blockId drift, an id-minting bug — so refusing it would 404 a
|
||||
// live public endpoint in exchange for closing no takedown path.
|
||||
// `tunnel_lookup_failed`
|
||||
// — ALWAYS 403, on every route, and 🔴 `onApprovalLookupFailure`
|
||||
// DOES NOT COVER IT. That option is scoped to `lookup_failed`
|
||||
// alone, deliberately: its argument is that a REPLICA read we
|
||||
// cannot complete should not take down routes where refusing
|
||||
// removes no exposure. This verdict is a CACHE read failing on the
|
||||
// dev-tunnel re-check, and the app it guards is already
|
||||
// NOT-approved — so serving it would not be tolerating an
|
||||
// unknown, it would be serving a known non-approved app because a
|
||||
// cache was down. If you are adding a route that wants
|
||||
// lookup-failure tolerance, this is the row that does not bend.
|
||||
//
|
||||
// All three are COUNTED regardless, before any of them branches. The counter is the
|
||||
// All four are COUNTED regardless, before any of them branches. The counter is the
|
||||
// alerting signal and it must not depend on what the route then decided to do.
|
||||
//
|
||||
// 🔴 The revocation check above fails OPEN and `lookup_failed` here fails CLOSED. That
|
||||
@@ -1001,7 +1021,17 @@ export function withBlockScope(handler: NextApiHandler, opts: WithBlockScopeOpts
|
||||
const approval = await resolveRestApprovalVerdict(claims);
|
||||
if (approval !== 'ok' && approval !== 'dev_exempt') {
|
||||
recordBlockRestApprovalVerdict(approval);
|
||||
if (approval === 'not_approved') {
|
||||
if (approval === 'not_approved' || approval === 'tunnel_lookup_failed') {
|
||||
// 🔴 BOTH REFUSE 403, WITH THE SAME BODY — the split is for the COUNTER, which
|
||||
// has already recorded the distinct `reason=` two lines above. A bearer learning
|
||||
// that the dev-tunnel cache is down rather than that the app is not approved
|
||||
// would be an infrastructure oracle with no benefit to it.
|
||||
//
|
||||
// ⚠️ NOT ROUTED THROUGH `lookup_failed`, which is the reuse that would look
|
||||
// tidier: that verdict answers 503 and is SERVED on the routes declaring
|
||||
// `onApprovalLookupFailure: 'serve'`. A non-approved app must not be served on
|
||||
// any route because a CACHE read failed, and a cache fault must not be reported
|
||||
// as a replica fault. Refusing here keeps both halves honest.
|
||||
res.status(403).json({ error: 'app block is not approved' });
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -36,6 +36,7 @@ const {
|
||||
mockGetSessionUser,
|
||||
mockIsRevoked,
|
||||
mockListMyBlockWorkflows,
|
||||
mockGetActiveDevTunnel,
|
||||
} = vi.hoisted(() => ({
|
||||
mockIsAppBlocksEnabled: vi.fn(),
|
||||
mockVerifyBlockToken: vi.fn(),
|
||||
@@ -45,6 +46,7 @@ const {
|
||||
mockGetSessionUser: vi.fn(),
|
||||
mockIsRevoked: vi.fn(),
|
||||
mockListMyBlockWorkflows: vi.fn(),
|
||||
mockGetActiveDevTunnel: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('~/server/services/app-blocks-flag', () => ({
|
||||
@@ -68,6 +70,16 @@ vi.mock('~/server/middleware/block-scope.middleware', () => ({
|
||||
vi.mock('~/server/services/block-revocation.service', () => ({
|
||||
BlockRevocation: { isRevoked: (...a: unknown[]) => mockIsRevoked(...a) },
|
||||
}));
|
||||
// The dev-tunnel re-check the approval predicate performs for a dev token on a REAL,
|
||||
// NOT-approved row. Reached through `await import(...)` inside the predicate — stubbed at
|
||||
// the specifier, which intercepts the dynamic form identically. `importOriginal` spread
|
||||
// rather than a hand-listed factory: this module has many other exports, and a factory
|
||||
// naming only one of them fails to LOAD as soon as anything in the graph imports a second,
|
||||
// with a green typecheck and an error far from the change.
|
||||
vi.mock('~/server/services/blocks/dev-tunnel.service', async (importOriginal) => ({
|
||||
...((await importOriginal()) as Record<string, unknown>),
|
||||
getActiveDevTunnel: (...a: unknown[]) => mockGetActiveDevTunnel(...a),
|
||||
}));
|
||||
vi.mock('~/server/services/blocks/block-workflows.service', () => ({
|
||||
listMyBlockWorkflows: (...a: unknown[]) => mockListMyBlockWorkflows(...a),
|
||||
upsertBlockWorkflowOnSubmit: vi.fn(),
|
||||
@@ -130,6 +142,8 @@ import { TokenScope } from '~/shared/constants/token-scope.constants';
|
||||
import { dbMock } from '~/__tests__/mocks/db.mock';
|
||||
|
||||
const mockDbRead = dbMock.dbRead;
|
||||
/** The OWNER lookup the approval predicate performs in its dev + non-approved branch. */
|
||||
const mockOauthFindUnique = mockDbRead.oauthClient.findUnique;
|
||||
|
||||
function validClaims(over: Record<string, unknown> = {}) {
|
||||
return {
|
||||
@@ -180,6 +194,13 @@ beforeEach(() => {
|
||||
// below is attributable to the one condition that test flips.
|
||||
mockIsRevoked.mockResolvedValue(false);
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ id: 'apb_test', status: 'approved' });
|
||||
// The app's OWNER, matching `validClaims().sub` (`user:42`), so the ownership belt is
|
||||
// satisfied by default and every refusal below stays attributable to the one condition
|
||||
// its test flips. Resolved separately from the row — see the predicate for why it is not
|
||||
// a nested relation select.
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 42 });
|
||||
// Default world: NO active dev tunnel — the exempting condition is opted INTO.
|
||||
mockGetActiveDevTunnel.mockResolvedValue(null);
|
||||
});
|
||||
|
||||
describe('bridge guard — revocation', () => {
|
||||
@@ -251,22 +272,6 @@ describe('bridge guard — approved status', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The one exemption, and the reason it is not a hole: `/api/v1/block-tokens`'s
|
||||
* `tryDevTunnelOwnedNonApprovedMint` mints a dev token carrying the app's REAL ids for
|
||||
* an app that is deliberately NOT approved — a suspended/pending/deprecated app stays
|
||||
* runnable by its OWNER in the owner's own dev tunnel (self-bound, forced-SFW,
|
||||
* budget-capped, tunnel-gated, never public). Enforcing the approved status on a `dev`
|
||||
* token would break that documented path. Revocation still binds — see below.
|
||||
*/
|
||||
it('skips the approved check for a dev token, which may legitimately be non-approved', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ id: 'apb_test', status: 'suspended' });
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).resolves.toMatchObject({
|
||||
blue: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it('still revokes a dev token — the exemption is the approved check only', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockIsRevoked.mockResolvedValue(true);
|
||||
@@ -277,6 +282,142 @@ describe('bridge guard — approved status', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE DEV-TOKEN EXEMPTION, ON THE BRIDGE (clawgate #571).
|
||||
*
|
||||
* This block replaces a single test that read *"skips the approved check for a dev token,
|
||||
* which may legitimately be non-approved"* and asserted a 200 for `dev: true` against a
|
||||
* `suspended` row. That test was true about the code and wrong about the property: the
|
||||
* `dev` claim is stamped by SIX mint paths, of which only three may legitimately run a
|
||||
* non-approved app, so the guard exempted all six — for the 4h dev lifetime, 16× the
|
||||
* 900s default, on all fifteen bridge procedures.
|
||||
*
|
||||
* WHAT IS PINNED HERE is the split, on the surface the card is about. The fixture subject
|
||||
* is `user:42` throughout (`validClaims`), so `app.userId` is what moves between the
|
||||
* owner and non-owner cases, and the dev tunnel is what moves between populations A and E.
|
||||
* Every case below drives a REAL bridge procedure through the real guard, and each names
|
||||
* the population it stands for so a future reader can tell coverage from decoration.
|
||||
*/
|
||||
describe('bridge guard — the dev-token populations', () => {
|
||||
it('POPULATION E: owner + ACTIVE dev tunnel on a suspended app still drives the bridge', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ status: 'suspended' });
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 42 });
|
||||
mockGetActiveDevTunnel.mockResolvedValue({ sessionId: 's', userId: 42, blockId: 'blk_test' });
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).resolves.toMatchObject({
|
||||
blue: 1,
|
||||
});
|
||||
// 🔴 THE POSITIVE HALF OF THE SEAM, asserted HERE rather than borrowed from the
|
||||
// predicate's own suite. The predicate reaches `getActiveDevTunnel` through
|
||||
// `await import(...)`; if that dynamic import ever escaped this file's `vi.mock` the
|
||||
// real redis-backed function would run, return null, and this test would fail — but
|
||||
// only this assertion proves the mocked one was CALLED, with the owner id resolved
|
||||
// from the row and the token's own blockId, never anything a caller could choose.
|
||||
// `appId` and `blockId` are deliberately different strings, so an argument swap dies.
|
||||
expect(mockGetActiveDevTunnel).toHaveBeenCalledWith(42, 'blk_test');
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE DEFECT. Population A — a `dev:live` token whose mint REQUIRED
|
||||
* `status: 'approved'`, still live after a moderator suspension. Identical in every
|
||||
* respect to the test above except the absent dev tunnel, which is the only thing that
|
||||
* ever separated it from population E.
|
||||
*/
|
||||
it('POPULATION A: owner with NO dev tunnel is REFUSED on a suspended app', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ status: 'suspended' });
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 42 });
|
||||
mockGetActiveDevTunnel.mockResolvedValue(null);
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).rejects.toMatchObject({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'app block is not approved',
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The same refusal on a SECOND procedure, because the exemption was never per-proc —
|
||||
* it sat in the one guard all fifteen go through, so pinning one proc would understate
|
||||
* both the defect and the fix.
|
||||
*/
|
||||
it('POPULATION A: the refusal is the guard, not the procedure — listMyWorkflows too', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ status: 'suspended' });
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 42 });
|
||||
mockGetActiveDevTunnel.mockResolvedValue(null);
|
||||
await expect(caller().listMyWorkflows({ blockToken: 't' })).rejects.toMatchObject({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'app block is not approved',
|
||||
});
|
||||
});
|
||||
|
||||
it('a dev token whose subject is not the CURRENT owner is refused, tunnel or not', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ status: 'suspended' });
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 99 });
|
||||
mockGetActiveDevTunnel.mockResolvedValue({ sessionId: 's', userId: 42, blockId: 'blk_test' });
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).rejects.toMatchObject({
|
||||
code: 'FORBIDDEN',
|
||||
message: 'app block is not approved',
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 CRITERION 5 — the moderator review sandbox, on the bridge. A run-for-real review
|
||||
* token belongs to a MODERATOR, not the owner, and names a pending app; it must keep
|
||||
* working, and it must do so from its own signed claim rather than by accident. Every
|
||||
* other exempting condition is absent here: a suspended row owned by someone else, and
|
||||
* no dev tunnel.
|
||||
*/
|
||||
it('POPULATION F′: a run-for-real REVIEW token still drives the bridge on a non-approved app', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true, reviewRunForReal: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ status: 'suspended' });
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 99 });
|
||||
mockGetActiveDevTunnel.mockResolvedValue(null);
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).resolves.toMatchObject({
|
||||
blue: 1,
|
||||
});
|
||||
// From the claim alone — the row is never read.
|
||||
expect(mockDbRead.appBlock.findUnique).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* POPULATIONS B / C / D / F — the synthetic-id mints. No backing row, so nothing to be
|
||||
* approved; on the bridge this is the branch that would otherwise answer NOT_FOUND and
|
||||
* silently kill the pending / local-manifest / ephemeral-tunnel sandboxes.
|
||||
*/
|
||||
it('POPULATIONS B–D/F: a dev token with NO backing row still drives the bridge', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue(null);
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).resolves.toMatchObject({
|
||||
blue: 1,
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The corresponding NON-dev case is unchanged and stays a 404 — pinned next to its dev
|
||||
* sibling because the two verdicts for a missing row now diverge inside one function,
|
||||
* and a reader comparing them needs both in view.
|
||||
*/
|
||||
it('a NON-dev token with no backing row is still NOT_FOUND, not exempt', async () => {
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue(null);
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).rejects.toMatchObject({
|
||||
code: 'NOT_FOUND',
|
||||
});
|
||||
});
|
||||
|
||||
it('an APPROVED app with a dev token needs no exemption and never looks for a tunnel', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(validClaims({ dev: true }));
|
||||
mockDbRead.appBlock.findUnique.mockResolvedValue({ status: 'approved' });
|
||||
mockOauthFindUnique.mockResolvedValue({ userId: 42 });
|
||||
await expect(caller().getMyBuzzBalance({ blockToken: 't' })).resolves.toMatchObject({
|
||||
blue: 1,
|
||||
});
|
||||
// The cost claim in the predicate's docblock: only the dev + real-row + NOT-approved
|
||||
// path pays the tunnel lookup.
|
||||
expect(mockGetActiveDevTunnel).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('bridge guard — token validity', () => {
|
||||
it('401s an unverifiable token, before either new check runs', async () => {
|
||||
mockVerifyBlockToken.mockResolvedValue(null);
|
||||
|
||||
@@ -145,6 +145,17 @@ export async function resolveSharedContext(
|
||||
});
|
||||
}
|
||||
|
||||
// 🔴 THE STRICTEST OF THE THREE RESOLVERS THAT READ THIS ROW, AND DELIBERATELY SO.
|
||||
// The other two are `resolveAppBlockApprovalVerdict`
|
||||
// (`~/server/services/blocks/block-approval.service`), the shared predicate for the REST
|
||||
// middleware and the tRPC bridge, which exempts a run-for-real review token, a token
|
||||
// with no backing row, and an owner with a live dev tunnel; and `resolveStorageContext`
|
||||
// (`apps.router`), which exempts only the run-for-real review token. This one exempts
|
||||
// NOTHING, because shared storage is cross-user, app-global state and the review mint
|
||||
// never grants `apps:storage:shared:*` at all — so there is no case here to exempt, not
|
||||
// a disagreement about what approval means. Do not "align" these three without deciding
|
||||
// it; the reconciliation and its rationale are ledgered, and enforced on both growth and
|
||||
// shrink, in `src/server/services/__tests__/no-unguarded-block-rest-token.test.ts`.
|
||||
const block = await dbRead.appBlock.findUnique({
|
||||
where: { appId_blockId: { appId: claims.appId, blockId: claims.blockId } },
|
||||
select: { id: true, status: true },
|
||||
|
||||
@@ -386,6 +386,17 @@ async function resolveStorageContext(
|
||||
};
|
||||
}
|
||||
|
||||
// 🔴 STRICTER THAN THE SHARED PREDICATE, AND THAT IS A STATEMENT ABOUT THIS TARGET
|
||||
// RATHER THAN ABOUT APPROVAL. The run-for-real branch above is this resolver's ONLY
|
||||
// exemption. `resolveAppBlockApprovalVerdict`
|
||||
// (`~/server/services/blocks/block-approval.service`) — the shared predicate behind the
|
||||
// REST middleware and the tRPC bridge — also exempts a token with no backing row and an
|
||||
// owner running a suspended app in their own live dev tunnel; neither can apply here,
|
||||
// because per-user KV has to resolve to a REAL Postgres schema and a plain dev token
|
||||
// names none. `resolveSharedContext` (`apps-shared.router`) is stricter still and
|
||||
// exempts nothing. Three rules, one policy plus two structural narrowings — ledgered
|
||||
// with their rationales, and enforced on growth and shrink, in
|
||||
// `src/server/services/__tests__/no-unguarded-block-rest-token.test.ts`.
|
||||
const block = await dbRead.appBlock.findUnique({
|
||||
where: { appId_blockId: { appId: claims.appId, blockId: claims.blockId } },
|
||||
select: { id: true, status: true },
|
||||
|
||||
@@ -792,11 +792,11 @@ const BACKING_ROW_LOOKUP_RE = /\bappId_blockId\b/;
|
||||
|
||||
const BACKING_ROW_LOOKUP_LEDGER: Record<string, string> = {
|
||||
'src/server/services/blocks/block-approval.service.ts':
|
||||
'THE PREDICATE. The one place the row is resolved from token claims and compared against `approved`. Both halves of the runtime — withBlockScope (REST) and assertAppBlockApproved (the tRPC bridge) — resolve their verdict here. A new approval check belongs in this file or calling it, not beside it.',
|
||||
'THE PREDICATE. The one place the row is resolved from token claims and compared against `approved`. Both halves of the runtime — withBlockScope (REST) and assertAppBlockApproved (the tRPC bridge) — resolve their verdict here. Its exemptions are THREE named cases, not the bare `dev` claim (clawgate #571): a signed reviewRunForReal review token, a synthetic id with no backing row, and the owner-dev-tunnel case re-derived from the row owner plus an ACTIVE tunnel. A new approval check belongs in this file or calling it, not beside it.',
|
||||
'src/server/routers/apps.router.ts':
|
||||
'resolveStorageContext — per-user KV. Reads the row and refuses a non-approved one itself, and is STRICTER than the predicate by construction: it exempts only reviewRunForReal, not `dev` generally, because per-user KV must resolve to a real Postgres schema and a plain dev token names none. Folding it into the predicate would widen an exemption it deliberately does not have.',
|
||||
'resolveStorageContext — per-user KV. Reads the row and refuses a non-approved one itself, and is STRICTER than the predicate: it exempts ONLY reviewRunForReal, because per-user KV must resolve to a real Postgres schema and a plain dev token names none. Since clawgate #571 the predicate answers reviewRunForReal first too, so the two now agree about the review sandbox and differ by exactly the owner-dev-tunnel case — correctly, since a suspended app being debugged in its owner tunnel has a page to render but no per-user KV schema to write. Folding this into the predicate would widen an exemption it deliberately does not have.',
|
||||
'src/server/routers/apps-shared.router.ts':
|
||||
'resolveSharedContext — shared, app-global KV. Same shape, exempts NOTHING: shared storage is cross-user state and run-for-real never grants apps:storage:shared:* at all, so there is no case to exempt. Also stricter than the predicate, for a reason about its target rather than about approval.',
|
||||
'resolveSharedContext — shared, app-global KV. Same shape, exempts NOTHING: shared storage is cross-user state and run-for-real never grants apps:storage:shared:* at all, so there is no case to exempt. Stricter than both the predicate and resolveStorageContext, for a reason about its target rather than about approval.',
|
||||
'src/pages/api/v1/developer/block-manifests.ts':
|
||||
'NOT an approval check. The developer manifest-upload path, keyed on an authenticated appId rather than on token claims: it reads the existing row to refuse server-controlled trustTier/renderMode changes, then upserts on the same unique. It gates on trust tier, never on status.',
|
||||
};
|
||||
|
||||
@@ -63,21 +63,22 @@ export function isSubjectScopedInstanceId(blockInstanceId: string): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* The token `sub` a given userId mints as. THE ONE PLACE this format is written on the
|
||||
* WRITE side — the read side passes `claims.sub` verbatim, so if these two spellings
|
||||
* ever disagree the scoped marker silently refuses nobody.
|
||||
* The token `sub` a given userId mints as.
|
||||
*
|
||||
* Pinned by `__tests__/subject-key-round-trip.test.ts`, which feeds this through
|
||||
* `parseSubjectUserId` — the READ side's parser — and back. An earlier version of this
|
||||
* sentence claimed that suite already existed; it did not, and the only thing holding the
|
||||
* spelling was a hand-typed `:user:<id>:` literal in
|
||||
* `services/__tests__/ban-revokes-block-instances.test.ts`. That literal still
|
||||
* independently reds on a spelling change and is worth keeping for exactly that reason,
|
||||
* but it pins the KEY, not this function.
|
||||
* ⚠️ MOVED to `~/server/services/block-token-subject` and re-exported here so this
|
||||
* module's existing importers are unchanged. The docblock that used to sit on the
|
||||
* definition claimed this was "THE ONE PLACE this format is written on the WRITE side",
|
||||
* and that was FALSE even when written: `block-token.service.ts`'s mint open-coded the
|
||||
* same template, so the ban writer and the thing that actually stamps every JWT were two
|
||||
* copies agreeing by coincidence. clawgate #571's approval guard would have been a
|
||||
* third. The claim is now true, of the leaf — see that module for why it is a leaf.
|
||||
*
|
||||
* Still pinned by `__tests__/subject-key-round-trip.test.ts`, which feeds it through
|
||||
* `parseSubjectUserId` — the READ side's parser — and back; and the hand-typed
|
||||
* `:user:<id>:` literal in `services/__tests__/ban-revokes-block-instances.test.ts`
|
||||
* independently reds on a spelling change, pinning the KEY rather than this function.
|
||||
*/
|
||||
export function subjectForUserId(userId: number): string {
|
||||
return `user:${userId}`;
|
||||
}
|
||||
export { subjectForUserId } from '~/server/services/block-token-subject';
|
||||
|
||||
/**
|
||||
* Per-blockInstanceId token revocation, written when an install is uninstalled,
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* THE BLOCK-TOKEN SUBJECT FORMAT — one spelling, for both the write side and the read
|
||||
* side, in a module with NO imports.
|
||||
*
|
||||
* A block token's `sub` is `user:<id>` for an authenticated subject and the literal
|
||||
* `anon` otherwise. That is a wire format shared by parties that must never disagree: the
|
||||
* MINT stamps it, the VERIFIER validates it, the revocation writer builds a key from it,
|
||||
* and the approval guard compares an app's owner against it. If any two of those spell it
|
||||
* differently the failure is silent in the dangerous direction — a marker that refuses
|
||||
* nobody, or an owner locked out of their own app — because every one of them is
|
||||
* individually correct and only the PAIR is wrong.
|
||||
*
|
||||
* 🔴 WHY A ZERO-IMPORT LEAF RATHER THAN A HOME IN ONE OF THE CONSUMERS. Every consumer is
|
||||
* a module some other consumer already imports, so whichever one owned this would create
|
||||
* a cycle for the rest — `block-scope.middleware` statically imports
|
||||
* `blocks/block-approval.service`, so the approval guard cannot import the parser back
|
||||
* out of the middleware. Worse, three of the candidate homes are wholesale-`vi.mock`ed
|
||||
* across the suite (`block-revocation.service` in twelve files, exporting only
|
||||
* `BlockRevocation`), so an import from there resolves to `undefined` in exactly the
|
||||
* suites that run the real guard — a green test over a broken comparison. A leaf nobody
|
||||
* mocks is the only shape that avoids both. Same reasoning, and the same shape, as
|
||||
* `block-token-lifetimes.ts`.
|
||||
*
|
||||
* 🔴 HISTORY, BECAUSE THE UNIQUENESS CLAIM HAS ALREADY BEEN FALSE ONCE.
|
||||
* `subjectForUserId` used to live in `block-revocation.service.ts` under a docblock
|
||||
* reading "THE ONE PLACE this format is written on the WRITE side". It was not: the mint
|
||||
* in `block-token.service.ts` open-coded the same template, and clawgate #571's approval
|
||||
* guard added a third. Each copy was pinned by its OWN hand-typed literal rather than to
|
||||
* the others, so the suite could not see them diverge — changing the mint's encoding
|
||||
* would have left the approval guard's tests green (they hand-type `sub: 'user:42'`
|
||||
* beside `app: { userId: 42 }`, i.e. they prove the template matches itself) while
|
||||
* production refused every owner-dev-tunnel token. Consolidating here is what makes the
|
||||
* mint and the guard produce the same string BY CONSTRUCTION rather than by coincidence,
|
||||
* which is a property no additional test could have bought.
|
||||
*
|
||||
* A new spelling of this format belongs in this file, not beside its call site.
|
||||
*/
|
||||
|
||||
/**
|
||||
* The canonical authenticated-subject shape. Leading-zero, empty and oversized ids are
|
||||
* all rejected, so a `sub` that passes this can be compared as a STRING against
|
||||
* {@link subjectForUserId}'s output without a parse step and without numeric coercion.
|
||||
*/
|
||||
export const USER_SUB_RE = /^user:[1-9][0-9]{0,11}$/;
|
||||
|
||||
/** The literal anonymous subject. Never equal to any {@link subjectForUserId} output. */
|
||||
export const ANON_SUBJECT = 'anon';
|
||||
|
||||
/** Every `sub` a token we signed may legitimately carry. */
|
||||
export function isValidSubject(sub: string): boolean {
|
||||
return sub === ANON_SUBJECT || USER_SUB_RE.test(sub);
|
||||
}
|
||||
|
||||
/**
|
||||
* The token `sub` a given userId mints as.
|
||||
*
|
||||
* Callers that hold a userId and want to know whether it is the token's subject should
|
||||
* compare FORWARD — `claims.sub === subjectForUserId(id)` — rather than parsing the
|
||||
* claim. `verifyBlockToken` has already run {@link isValidSubject}, so by the time any
|
||||
* guard sees `sub` it is `anon` or a canonical `user:<id>`, which makes the forward
|
||||
* comparison exactly equivalent to parsing and one step shorter.
|
||||
*/
|
||||
export function subjectForUserId(userId: number): string {
|
||||
return `user:${userId}`;
|
||||
}
|
||||
@@ -3,6 +3,7 @@ import { SignJWT } from 'jose';
|
||||
import { env } from '~/env/server';
|
||||
import { redis, REDIS_KEYS } from '~/server/redis/client';
|
||||
import { BLOCK_TOKEN_LIFETIMES_SECONDS } from '~/server/services/block-token-lifetimes';
|
||||
import { ANON_SUBJECT, subjectForUserId } from '~/server/services/block-token-subject';
|
||||
|
||||
// L7 (audit-10): shared issuer/audience constants exported for the
|
||||
// middleware so a typo in one place can't desynchronize sign-vs-verify.
|
||||
@@ -248,7 +249,12 @@ export class BlockTokenService {
|
||||
? BLOCK_TOKEN_LIFETIMES_SECONDS.settings
|
||||
: BLOCK_TOKEN_LIFETIMES_SECONDS.default;
|
||||
const exp = iat + lifetime;
|
||||
const sub = input.userId == null ? 'anon' : `user:${input.userId}`;
|
||||
// Built through the shared encoder rather than open-coded. This template used to be
|
||||
// written here AND in `block-revocation.service` (whose docblock claimed to be "THE
|
||||
// ONE PLACE this format is written on the WRITE side" while this line existed), and
|
||||
// every consumer pinned its own copy with its own literal — so nothing in the suite
|
||||
// could see them diverge. See `block-token-subject.ts`.
|
||||
const sub = input.userId == null ? ANON_SUBJECT : subjectForUserId(input.userId);
|
||||
|
||||
const claims: Record<string, unknown> = {
|
||||
blockId: input.blockId,
|
||||
|
||||
@@ -123,6 +123,24 @@ const GATE_LEDGER: Record<string, string> = {
|
||||
'(D1) and none is meaningful here — this is not a gate a caller passes through, it ' +
|
||||
'is a payee resolution. If collaborator revenue-sharing is ever specified it belongs ' +
|
||||
'as an explicit split on top of this row, not as a widened owner lookup.',
|
||||
'src/server/services/blocks/block-approval.service.ts':
|
||||
'resolveAppBlockApprovalVerdict resolves the app owner — `oauthClient.findUnique` on ' +
|
||||
'`claims.appId`, read as `app?.userId` — to decide whether a `dev` token may bypass ' +
|
||||
'the approved-status check on a REAL, NOT-approved row: the ' +
|
||||
'owner-dev-tunnel case (clawgate #571). It is a SEPARATE query rather than a nested ' +
|
||||
'select on the row read, because without `relationJoins` a nested relation is a ' +
|
||||
'second round trip anyway and would bill every bridge call and every REST request ' +
|
||||
'for a column only this branch consults. DELIBERATELY OWNER-ONLY, and the reason is ' +
|
||||
'that it is a MIRROR rather than a policy of its own: the only mint that can issue ' +
|
||||
'such a token, `resolveOwnedNonApprovedPageBlock`, resolves `where: { app: { userId ' +
|
||||
'} }` — owner-only, not widened to seats. Widening THIS read to ACCEPTED ' +
|
||||
'collaborators would exempt a class of token the mint can never produce, i.e. it ' +
|
||||
'would only ever loosen the gate for a stale token, never enable a real editor ' +
|
||||
'workflow. If collaborator dev tunnels are ever specified, the MINT is what changes ' +
|
||||
'first and this read follows it — never the other way round. NO mod bypass (D1): a ' +
|
||||
'moderator reviewing a non-approved app already has its own exemption, the signed ' +
|
||||
'`reviewRunForReal` claim answered before this read, so a mod override here would ' +
|
||||
'be a second, weaker path to the same thing.',
|
||||
'src/server/services/blocks/app-analytics.service.ts':
|
||||
'getOwnedAppBlocks resolves the permitted-id SET (owned + seated) instead of ' +
|
||||
'`app: { userId }`. Safe to widen HERE because every downstream aggregate filters ' +
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { dbRead } from '~/server/db/client';
|
||||
import type { BlockTokenClaims } from '~/server/middleware/block-scope.middleware';
|
||||
import { subjectForUserId } from '~/server/services/block-token-subject';
|
||||
|
||||
/**
|
||||
* WHY THIS IS ITS OWN MODULE rather than a private function inside
|
||||
@@ -84,44 +85,93 @@ import type { BlockTokenClaims } from '~/server/middleware/block-scope.middlewar
|
||||
* suspended app can never obtain a NEW token. What was open was the tail of the tokens it
|
||||
* already held.
|
||||
*
|
||||
* 🔴 THE TAIL THIS GATE CLOSES IS THE NON-DEV ONE: 900s default and 300s settings-scoped
|
||||
* (`src/server/services/block-token-lifetimes.ts`). The third class in that file — the
|
||||
* 4-HOUR DEV TOKEN — is the LONGEST-LIVED token there is and this gate does NOT close it,
|
||||
* because `claims.dev === true` is exempted six lines below, on purpose. Do not size the
|
||||
* residual risk by reading the three lifetimes as one closed set: the largest of them is
|
||||
* the one deliberately left open, and it is left open because the moderator review sandbox
|
||||
* and the owner dev-tunnel both depend on it (the exemption paragraph below is the
|
||||
* argument, and the containment that stands in for this gate on that class). Fifteen
|
||||
* minutes of Buzz leaving an account through an app a moderator has just taken down — on
|
||||
* an ordinary, non-dev token — is the case this is for.
|
||||
* 🔴 THE TAIL THIS GATE CLOSES NOW INCLUDES MOST OF THE DEV ONE. It used to close only
|
||||
* the non-dev tail — 900s default and 300s settings-scoped
|
||||
* (`src/server/services/block-token-lifetimes.ts`) — because `claims.dev === true`
|
||||
* short-circuited the whole check, leaving the 4-HOUR DEV TOKEN, the longest-lived token
|
||||
* there is, entirely outside it: 16× the default window, on the class with the widest
|
||||
* scopes. Fifteen minutes of Buzz leaving an account through an app a moderator has just
|
||||
* taken down is the case this gate was built for; four hours of the same on a dev token
|
||||
* was the case it did not reach (clawgate #571). The exemption is now conditioned rather
|
||||
* than unconditional — see the predicate.
|
||||
*
|
||||
* 🔴 THE ONE EXEMPTION — `claims.dev === true`, the same predicate `assertAppBlockApproved`
|
||||
* applies on the bridge, and chosen so the two paths cannot drift rather than re-decided
|
||||
* on its own merits. Run-for-real moderator REVIEW tokens are `dev: true`, and review is the one
|
||||
* surface that MUST work on a NON-approved app; the owner dev-tunnel mint (#3285) signs
|
||||
* a real `apb_` id for an app that is deliberately suspended/pending/deprecated so its
|
||||
* owner can diagnose it back into review; and the pending / local-manifest / review
|
||||
* mints sign a synthetic `pubreq_…` / `page_local_…` / `ephemeral-…` id with no backing
|
||||
* row to be approved at all. A gate without this exemption breaks all three. Dev tokens
|
||||
* remain revocable (every such mint stamps a revocable instance id, and the revocation
|
||||
* check at the call site is NOT exempted), mod/dev-cohort gated at mint, forced-SFW and
|
||||
* budget-capped.
|
||||
* 🔴 THE EXEMPTION, AND WHY IT IS THREE CASES RATHER THAN ONE BOOLEAN. `dev: true` is
|
||||
* stamped in exactly one place — `signDevScopedPageToken`, UNCONDITIONALLY — and that
|
||||
* function is reached by six mint paths, so the bare claim says only "one of six things".
|
||||
* The letters below are used by name in the code; this is their key, and it is the
|
||||
* population table the rest of this file refers to:
|
||||
*
|
||||
* ID MINT appId / appBlockId ROW? MINT NEEDS approved? VERDICT
|
||||
* A `dev-token` approved mode (`dev:live`) real / real `apb_` yes YES `ok` while approved; REFUSED once it is not, unless owner+tunnel
|
||||
* B `dev-token` pending mode `pending-…` / `pubreq_…` no n/a exempt — no row
|
||||
* C `dev-token` local-manifest mode `local-…` / `page_local_…` no n/a exempt — no row
|
||||
* D `tryDevTunnelScopedMint` (ephemeral tunnel) `ephemeral-…` (both) no n/a exempt — no row
|
||||
* E `tryDevTunnelOwnedNonApprovedMint` real / real `apb_` yes NO — requires NOT approved exempt IFF owner AND active tunnel
|
||||
* F `mintReviewBlockToken` (render-only) `pending-…` / `pubreq_…` no requires `pending` exempt — no row
|
||||
* F′ `mintReviewBlockToken` (run-for-real) as F, + `reviewRunForReal` no requires `pending` exempt — from the claim, pre-read
|
||||
*
|
||||
* A and E are CLAIM-IDENTICAL: same id shapes, same `dev: true`, both owner-held, no
|
||||
* distinguishing field. What separates them is not the token but the preconditions their
|
||||
* mints enforce — E requires an ACTIVE dev tunnel, A does not — which is why the guard
|
||||
* re-derives those rather than reading a flag.
|
||||
*
|
||||
* ⚠️ ONE CASE THE TABLE DOES NOT CAPTURE: an A-class token held by an owner who happens to
|
||||
* have a live tunnel for the same slug IS exempted. That is the deliberate mirror (the
|
||||
* same owner could mint an E token for the same app in the same state), but A and E are
|
||||
* not scope-identical — A clamps against `DEV_TOKEN_SCOPE_ALLOWLIST`, which includes
|
||||
* `apps:storage:read|write`, while E's tunnel allowlist withholds them. The delta is
|
||||
* closed one layer down rather than here: `resolveStorageContext` exempts only
|
||||
* `reviewRunForReal`, so those storage scopes stay inert on a non-approved app.
|
||||
*
|
||||
* 🔴 WHAT THIS DELIBERATELY DOES NOT RE-CHECK, so the justification is not overstated
|
||||
* again. The mint-time belts also include the author / dev-tunnel Flipt flags, forced-SFW,
|
||||
* the dev budget cap, and the clamp of scopes to the last moderator-approved snapshot
|
||||
* (`approvedScopes`). NONE of those are re-evaluated here. Two belts are re-checked; the
|
||||
* rest are containment, and are named as containment rather than as this gate's reasoning.
|
||||
*
|
||||
* ⚠️ AND THE CONTAINMENT CLAIM IS SCOPED, because an earlier draft of this paragraph
|
||||
* overstated it in exactly the way the card warned about. *For population E*, a
|
||||
* never-approved app cannot obtain `ai:write:budgeted`: its only scope source is
|
||||
* `clampTunnelDeclaredScopes(app.approvedScopes)`, `approvedScopes` is written only by the
|
||||
* mod-approval flow, and clamping `[]` cannot invent a scope. That is an E-path invariant,
|
||||
* NOT a property of dev tokens. B and C source the un-reviewed pending manifest and the
|
||||
* RAW CLIENT REQUEST BODY respectively, and D sources the tunnel session's declared
|
||||
* grants; none is clamped to any moderator-approved snapshot, and D's brand-new branch
|
||||
* strips spend only while `app-blocks-dev-tunnel-unsubmitted-spend` is OFF. Those three
|
||||
* are bounded instead by the bearer's own `AIServicesWrite` entitlement, self-bound spend,
|
||||
* the per-call dev cap and the per-user daily cap — and they are also the populations this
|
||||
* gate still exempts unconditionally, because they have no row an APPROVAL gate could ever
|
||||
* have decided on. That is the residual surface; it is not closed here and is not claimed
|
||||
* to be. Dev tokens remain revocable regardless — a separate check at each call site,
|
||||
* never exempted.
|
||||
*
|
||||
* 🔴 HOW THIS RECONCILES WITH THE OTHER TWO RESOLVERS, which apply visibly different
|
||||
* rules — they are not three policies, they are one policy plus two structural
|
||||
* narrowings, and reading them as policy disagreements is the mistake to avoid:
|
||||
* narrowings, and reading them as policy disagreements is the mistake to avoid. The gap
|
||||
* NARROWED with clawgate #571: this predicate now answers `reviewRunForReal` FIRST, which
|
||||
* is `resolveStorageContext`'s entire rule, so the three no longer disagree about the
|
||||
* review sandbox — only about how much more they refuse beyond it.
|
||||
* - `resolveStorageContext` (apps.router) exempts ONLY `reviewRunForReal`, not `dev`
|
||||
* generally. Per-user KV has to resolve to a real Postgres SCHEMA; a plain dev token
|
||||
* names no schema that exists, so there is nothing a wider exemption could route to.
|
||||
* It is therefore STRICTER than this gate by exactly the owner-dev-tunnel case, and
|
||||
* that is correct for its target: a suspended app's owner debugging in their tunnel
|
||||
* has a page to render, not a per-user KV schema to write.
|
||||
* - `resolveSharedContext` (apps-shared.router) exempts nothing. Shared storage is
|
||||
* cross-user, app-global state, and run-for-real never grants
|
||||
* `apps:storage:shared:*` at all — so again there is no case to exempt.
|
||||
* Both are STRICTER than this gate for a reason about their target, not about approval.
|
||||
* Both are STRICTER than this gate for a reason about their target, not about approval,
|
||||
* and each is ledgered with that rationale in
|
||||
* `src/server/services/__tests__/no-unguarded-block-rest-token.test.ts`.
|
||||
*
|
||||
* 🔴 POSTURE, and it is NOT uniform — read the two cases separately, because a single
|
||||
* "fail-closed" sentence over both was wrong in the direction that costs availability.
|
||||
*
|
||||
* - THE READ FAILED (`lookup_failed`) → FAIL-CLOSED, 503 on REST. A replica we cannot
|
||||
* - THE READ FAILED (`lookup_failed`) → FAIL-CLOSED, 503 on REST — ⚠️ on most
|
||||
* routes. It is ROUTE-DEPENDENT: the five declaring `onApprovalLookupFailure:
|
||||
* 'serve'` are SERVED instead, by their own choice. Stating it flat is the same
|
||||
* shape of wrong claim this file has now had to correct twice for the neighbouring
|
||||
* verdict, so it is spelled out rather than rounded off. A replica we cannot
|
||||
* reach leaves us unable to establish that the app is allowed to run at all. That is
|
||||
* the OPPOSITE of the revocation check one step earlier, deliberately:
|
||||
* `BlockRevocation.isRevoked` fails OPEN by construction — a Redis incident must not
|
||||
@@ -149,9 +199,19 @@ import type { BlockTokenClaims } from '~/server/middleware/block-scope.middlewar
|
||||
* two policies differ ON PURPOSE, which is why this module hands back a VERDICT.
|
||||
*
|
||||
* COST: one indexed `dbRead.appBlock.findUnique` on the `(appId, blockId)` unique, on
|
||||
* the replica, per block-JWT REST request. A dev token skips it entirely. The tRPC
|
||||
* bridge already pays exactly this per bridge call including `pollWorkflow`, so this is
|
||||
* the same bill on a lower-volume surface, not a new class of cost.
|
||||
* the replica, per block-JWT REST request. The tRPC bridge already pays exactly this per
|
||||
* bridge call including `pollWorkflow`, so this is the same bill on a lower-volume
|
||||
* surface, not a new class of cost.
|
||||
* ⚠️ "A DEV TOKEN SKIPS IT ENTIRELY" WAS TRUE UNTIL clawgate #571 AND IS NOT NOW. A dev
|
||||
* token now pays the same read as every other token — the skip WAS the hole, because
|
||||
* skipping the read is what made the verdict independent of the row's status. Exactly one
|
||||
* thing short-circuits ahead of it: a run-for-real review token, answered from its claims.
|
||||
* The read itself is UNCHANGED — still `select: { status: true }` — and that is
|
||||
* deliberate: the owner column is resolved inside the rare branch instead, because a
|
||||
* nested relation select would have been a second round trip on every request rather than
|
||||
* a wider row (no `relationJoins`; see the predicate). So the new cost is TWO lookups —
|
||||
* one `OauthClient` primary-key read and two sysRedis GETs — both reached ONLY on the dev
|
||||
* + real-row + NOT-approved path. No approved app and no non-dev token pays either.
|
||||
*
|
||||
* WHAT IT DOES NOT BUY, stated because the gate is uniform and the value is not. The
|
||||
* clearest case is `/api/v1/models/:id`: it is dual-auth, and the block-JWT branch
|
||||
@@ -162,39 +222,210 @@ import type { BlockTokenClaims } from '~/server/middleware/block-scope.middlewar
|
||||
* same argument one step weaker — their bodies are public but maturity-clamped per
|
||||
* token, so a block does not get strictly nothing extra there.
|
||||
*/
|
||||
export type AppBlockApprovalVerdict = 'ok' | 'dev_exempt' | 'not_approved' | 'not_found';
|
||||
export type AppBlockApprovalVerdict =
|
||||
| 'ok'
|
||||
| 'dev_exempt'
|
||||
| 'not_approved'
|
||||
| 'not_found'
|
||||
/**
|
||||
* 🔴 THE DEV-TUNNEL RE-CHECK COULD NOT BE COMPLETED. Refuses exactly like
|
||||
* `not_approved` on both callers — it is NOT a softer verdict — but it is a SEPARATE
|
||||
* one so the refusal is attributable.
|
||||
*
|
||||
* ⚠️ IT EXISTS BECAUSE A LOG WAS THE WRONG ANSWER HERE, AND THE REASON IS SPECIFIC TO
|
||||
* THIS DEPLOYMENT: **application-container logs are not collected**, which
|
||||
* `app-block-runtime.metrics.ts` states twice and uses as the basis for its own design
|
||||
* ("the `console.error` shape used elsewhere in the repo would be invisible to a later
|
||||
* investigator"). An earlier round of this change answered the silent-swallow problem
|
||||
* with a throttled `console.warn` and argued that log was the signal separating a cache
|
||||
* incident from the stale-token population. It is not — nobody can read it. Folding
|
||||
* these into `not_approved` therefore leaves a sysRedis fault looking exactly like the
|
||||
* narrowing working, on the one series anybody watches.
|
||||
*
|
||||
* NOT `lookup_failed`, which would be the lazy reuse: that verdict means the REPLICA
|
||||
* read failed, maps to 503, and is SERVED on the five routes declaring
|
||||
* `onApprovalLookupFailure: 'serve'`. Both would be wrong here — a cache fault blamed
|
||||
* on the database, and a non-approved app served on some routes.
|
||||
*/
|
||||
| 'tunnel_lookup_failed';
|
||||
|
||||
/**
|
||||
* THE PREDICATE. The only place in the App Blocks runtime that resolves the backing
|
||||
* `app_blocks` row from token claims and decides whether it is approved.
|
||||
*
|
||||
* 🔴 IT DOES NOT CATCH. A failed read propagates, so each caller keeps its OWN answer to
|
||||
* "what does an unreachable replica mean here" rather than inheriting one: REST converts
|
||||
* it to a 503 in `resolveRestApprovalVerdict` below, and the bridge lets it propagate as
|
||||
* it always has. Catching HERE would have silently changed the bridge's behaviour on a
|
||||
* replica incident — from the raw error it surfaces today to a swallowed one plus a log
|
||||
* line the bridge never emitted — which is exactly the kind of drift consolidating two
|
||||
* copies is supposed to prevent, not introduce.
|
||||
* 🔴 IT DOES NOT CATCH **THE ROW READS**. A failed `appBlock` or `oauthClient` read
|
||||
* propagates, so each caller keeps its OWN answer to "what does an unreachable replica
|
||||
* mean here" rather than inheriting one: REST converts it to a 503 in
|
||||
* `resolveRestApprovalVerdict` below, and the bridge lets it propagate as it always has.
|
||||
* Catching those HERE would silently change the bridge's behaviour on a replica incident —
|
||||
* from the raw error it surfaces today to a swallowed one plus a log line the bridge never
|
||||
* emitted — which is exactly the kind of drift consolidating two copies is supposed to
|
||||
* prevent, not introduce.
|
||||
*
|
||||
* ⚠️ THE DEV-TUNNEL RE-CHECK IS THE ONE EXCEPTION, AND THIS HEADING USED TO DENY IT
|
||||
* BLANKET-STYLE. That leg (clawgate #571) IS wrapped, and it does exactly what the
|
||||
* paragraph above calls the anti-pattern: swallows, logs a line the bridge never emitted,
|
||||
* and answers a verdict. The difference that makes it the right call there and the wrong
|
||||
* one here is the SUBJECT: an unreachable replica means "we cannot establish whether this
|
||||
* app may run at all", which is a different question per caller; an unreachable dev-tunnel
|
||||
* cache means "this owner has no live tunnel", which is the same answer everywhere and is
|
||||
* the fail-closed one. The exception is deliberate, it is argued at the `catch` itself,
|
||||
* and it is scoped to that one call — do not widen it to the reads.
|
||||
*/
|
||||
export async function resolveAppBlockApprovalVerdict(
|
||||
claims: BlockTokenClaims
|
||||
): Promise<AppBlockApprovalVerdict> {
|
||||
if (claims.dev === true) return 'dev_exempt';
|
||||
// POPULATION F′ — the moderator run-for-real review sandbox. The ONE population with a
|
||||
// purpose-built discriminator, and the ONE that must run a non-approved app while NOT
|
||||
// being its owner. Answered before the read because the review mint signs
|
||||
// `appId: pending-<ULID>`, which is not an `OauthClient.id` and so resolves to no row —
|
||||
// the read could only ever return `not_found`. (The `pubreq_` id everyone reaches for
|
||||
// when explaining this is the `appBlockId`, which this lookup never touches.)
|
||||
//
|
||||
// 🔴 `dev` IS REQUIRED ALONGSIDE IT, even though every mint that stamps
|
||||
// `reviewRunForReal` also stamps `dev`. The card this change answers is about keying
|
||||
// authorization on one signed boolean without narrowing it; taking `reviewRunForReal`
|
||||
// alone would be the same mistake one field over, and would make the exemption WIDER
|
||||
// than the one it replaced. `BlockTokenService.sign` accepts the field independently,
|
||||
// so the pairing is the only thing that closes that dimension, and it costs nothing.
|
||||
if (claims.dev === true && claims.reviewRunForReal === true) return 'dev_exempt';
|
||||
|
||||
const block = await dbRead.appBlock.findUnique({
|
||||
where: { appId_blockId: { appId: claims.appId, blockId: claims.blockId } },
|
||||
select: { status: true },
|
||||
});
|
||||
if (!block) return 'not_found';
|
||||
return block.status === 'approved' ? 'ok' : 'not_approved';
|
||||
|
||||
// POPULATIONS B / C / D / F — the synthetic-id mints (`pubreq_…`, `page_local_…`,
|
||||
// `ephemeral-…`). They sign an `appId` that is not an `OauthClient.id`, so the unique
|
||||
// resolves to nothing and there is no row to be approved. A non-dev token reaching
|
||||
// `not_found` is the separate false-positive channel documented above; it is unchanged.
|
||||
if (!block) return claims.dev === true ? 'dev_exempt' : 'not_found';
|
||||
|
||||
if (block.status === 'approved') return 'ok';
|
||||
|
||||
// From here the row EXISTS and is NOT approved.
|
||||
if (claims.dev !== true) return 'not_approved';
|
||||
|
||||
// 🔴 POPULATION A vs POPULATION E — the whole point of this function, and the one split
|
||||
// the `dev` boolean cannot make on its own. Both sign the app's REAL ids, both carry
|
||||
// `dev: true`, and their claim sets are otherwise identical:
|
||||
//
|
||||
// A — `/api/v1/blocks/dev-token` in approved mode. The mint REQUIRES
|
||||
// `status === 'approved'` (that file's "has no live deployment" refusal), so a
|
||||
// token of this class reaching a NON-approved row means the status flipped AFTER
|
||||
// it was minted — i.e. a moderator suspension, an owner unpublish, or a
|
||||
// re-submission. It has no standing claim on a non-approved app and must be
|
||||
// REFUSED. This is the 4h window this function exists to close.
|
||||
// E — `tryDevTunnelOwnedNonApprovedMint` (`/api/v1/block-tokens`). The mint
|
||||
// DELIBERATELY resolves `status: { not: 'approved' }`, so its whole purpose is to
|
||||
// keep a suspended / pending / deprecated app runnable by its OWNER inside the
|
||||
// OWNER'S OWN dev tunnel, to diagnose it back into review. It must keep working.
|
||||
//
|
||||
// What separates them is not the token — it is the two preconditions E's mint enforces
|
||||
// and A's does not. So this re-checks exactly those two, rather than trusting that they
|
||||
// held at mint time for a token that may be four hours old:
|
||||
//
|
||||
// 1. OWNERSHIP — the subject IS the app's owner. Free (the column above). Not merely
|
||||
// defensive: app ownership can TRANSFER, and a transferred-away app must not stay
|
||||
// drivable by the previous owner's outstanding dev token.
|
||||
// 2. AN ACTIVE DEV TUNNEL for (owner, blockId) — the precondition that makes E a
|
||||
// dev-tunnel affordance rather than a general un-suspend, and the only one of the
|
||||
// mint's belts that is both cheap to re-derive and actually discriminating. It
|
||||
// expires on its own (30m idle / 8h hard), so a token outliving the debugging
|
||||
// session it was minted for stops being exempt.
|
||||
//
|
||||
// 🔴 THE OWNER IS RESOLVED HERE, IN THE BRANCH, AND NOT AS A NESTED SELECT ON THE READ
|
||||
// ABOVE. The obvious spelling — `select: { status: true, app: { select: { userId } } }`
|
||||
// — reads like one widened row and is NOT: the schema's generator block enables only
|
||||
// `previewFeatures = ["metrics"]`, with no `relationJoins`, so Prisma has no join
|
||||
// strategy available and resolves a nested relation with a SECOND round trip. `appId`
|
||||
// is a REQUIRED relation, so unlike a nullable FK that second query cannot be skipped —
|
||||
// it would fire for every token whose row exists, i.e. on every bridge call including
|
||||
// the timer-driven `pollWorkflow` and on every block-JWT REST request, to read a column
|
||||
// only this branch consults. `claims.appId` IS the `OauthClient.id` (it is the FK the
|
||||
// unique is keyed on), so doing it here is the same primary-key lookup Prisma would
|
||||
// have issued, issued only when it is needed. The identical measurement for this
|
||||
// mechanism is recorded in `src/server/selectors/reaction.selector.ts`.
|
||||
const app = await dbRead.oauthClient.findUnique({
|
||||
where: { id: claims.appId },
|
||||
select: { userId: true },
|
||||
});
|
||||
// Compared against the canonical subject encoding rather than parsed. `subjectForUserId`
|
||||
// is the encoder the MINT itself uses, so the guard and the token are the same string by
|
||||
// construction rather than by two hand-typed templates agreeing — which they did not:
|
||||
// this comparison was a third copy of `user:<id>` until the leaf was extracted, and each
|
||||
// copy was pinned only by its own literal, so a suite could not see them diverge.
|
||||
// Comparing forward also avoids importing the PARSER from `block-scope.middleware`,
|
||||
// which imports THIS module. `anon` can never match a numeric owner, and a missing row
|
||||
// fails CLOSED.
|
||||
const ownerUserId = app?.userId;
|
||||
if (ownerUserId == null || claims.sub !== subjectForUserId(ownerUserId)) return 'not_approved';
|
||||
|
||||
// Dynamic import, deliberately: `dev-tunnel.service` pulls the k8s control-plane client
|
||||
// and the sysRedis surface, and this module is imported STATICALLY by the REST
|
||||
// middleware, which fronts 13 page routes. Loading it lazily keeps that graph out of
|
||||
// every one of those bundles — the same reason `tryDevTunnelOwnedNonApprovedMint` and
|
||||
// all eight `blocks.router` call sites import it this way. Reached only on the dev +
|
||||
// real-row + NOT-approved path.
|
||||
//
|
||||
// 🔴 WRAPPED, BECAUSE "IT CANNOT THROW" WAS ALMOST TRUE AND ALMOST IS NOT A POSTURE.
|
||||
// `getActiveDevTunnel` swallows a rejected read, a `withSysReadDeadline` timeout and a
|
||||
// JSON parse failure — but it attaches `.catch(() => null)` to the RESULT of
|
||||
// `sysRedis.get(...)`, so a SYNCHRONOUS throw from the client (the exact shape
|
||||
// `dev-tunnel.service` warns about twice in its own file) escapes it, as can the
|
||||
// dynamic import itself. Unwrapped, that escape does not fail closed: on REST
|
||||
// `resolveRestApprovalVerdict` catches it as `lookup_failed` → 503, attributing a cache
|
||||
// fault to the replica read and pointing an incident at the wrong subsystem, and on the
|
||||
// bridge it surfaces as a raw internal error instead of a refusal. So the posture is
|
||||
// written rather than inherited.
|
||||
//
|
||||
// FAILING CLOSED HERE IS THE OPPOSITE OF THE REVOCATION CHECK ONE STEP EARLIER, which
|
||||
// fails OPEN by construction, and of the `lookup_failed` case above. That is deliberate:
|
||||
// the population that loses is owners debugging an app that is ALREADY suspended,
|
||||
// pending or deprecated. Nothing user-facing is served by a non-approved app either way,
|
||||
// so the cost is a degraded developer surface during an incident, not an outage.
|
||||
try {
|
||||
const { getActiveDevTunnel } = await import('~/server/services/blocks/dev-tunnel.service');
|
||||
// ⚠️ WHAT ENDS A SESSION EARLY IS THE REAPER, NOT THIS CALL — and its idle clock is
|
||||
// refreshed by ENTRY-document loads only. `dev-tunnel-gate` returns before stamping
|
||||
// `lastActivityAt` on the websocket and subresource branches, so HMR traffic and the
|
||||
// block's own XHR do not count as activity. An owner with the page open and no iframe
|
||||
// re-navigation for 30 minutes therefore loses this exemption MID-SESSION, and the
|
||||
// re-mint does not rescue them because the mint requires the same tunnel. That is the
|
||||
// intended direction — an exemption that outlives the debugging session it was minted
|
||||
// for is the thing this change exists to stop — but the failure is silent, so it is
|
||||
// recorded here rather than left for someone to rediscover from a support ticket.
|
||||
return (await getActiveDevTunnel(ownerUserId, claims.blockId)) ? 'dev_exempt' : 'not_approved';
|
||||
} catch (err) {
|
||||
// 🔴 ITS OWN VERDICT, NOT `not_approved`. Both refuse identically, so this is not a
|
||||
// softer outcome — it is an ATTRIBUTABLE one. Sharing `not_approved` would put a
|
||||
// sysRedis fault on the same series as every legitimate stale-token refusal, i.e. on
|
||||
// the one signal this whole change ships to be watched on, where it would read as the
|
||||
// narrowing working. The log below is kept for environments that collect container
|
||||
// logs; on THIS deployment they are not collected, so the counter is the signal —
|
||||
// ⚠️ ON REST. This predicate is shared, and the bridge caller records NO verdict
|
||||
// counter at all, so on that surface the unreadable log is still all there is. See
|
||||
// `tunnelFailureLog` for the full statement; do not read this line as "observable
|
||||
// everywhere" just because you are standing in the shared function.
|
||||
tunnelFailureLog.warn(err);
|
||||
return 'tunnel_lookup_failed';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* THE REST POLICY over that verdict: identical, plus a fail-closed `lookup_failed` for a
|
||||
* read that threw. `withBlockScope` maps the result onto status codes, and the mapping is
|
||||
* not "ok passes, everything else refuses": `ok` and `dev_exempt` serve, `not_approved`
|
||||
* (403) and `lookup_failed` (503) refuse, and `not_found` is counted and SERVED. The
|
||||
* not "ok passes, everything else refuses": `ok` and `dev_exempt` serve; `not_approved`
|
||||
* and `tunnel_lookup_failed` refuse 403 on EVERY route; `lookup_failed` refuses 503 on
|
||||
* most but is SERVED on the five declaring `onApprovalLookupFailure: 'serve'`, so it is
|
||||
* the one row that cannot be read flat; and `not_found` is counted and SERVED. The
|
||||
* mapping lives at that call site rather than here, because the bridge maps the same
|
||||
* verdicts onto a different policy.
|
||||
*
|
||||
* ⚠️ THIS IS THE CANONICAL COPY — `block-scope.middleware` points here rather than
|
||||
* restating it ("one docblock, not two"). It has twice drifted behind the code it
|
||||
* describes, so if you change the mapping, change it HERE and check the table at that
|
||||
* call site has not quietly grown a second version.
|
||||
*/
|
||||
export async function resolveRestApprovalVerdict(
|
||||
claims: BlockTokenClaims
|
||||
@@ -227,36 +458,127 @@ export async function resolveRestApprovalVerdict(
|
||||
* line rate is this rate times the pod count, which is the intended bound (a per-pod signal
|
||||
* is what tells you whether the incident is partial or total), not an oversight.
|
||||
*
|
||||
* 🔴 THE COUNT IS NOT THE ALERTING SIGNAL. `civitai_app_block_rest_approval_verdicts_total{reason="lookup_failed"}`
|
||||
* is, and it is UNTHROTTLED — every failure increments it. This throttle only bounds the
|
||||
* prose. Do not add a metric here and do not read a suppressed log as a suppressed verdict.
|
||||
* 🔴 THE COUNT IS NOT THE ALERTING SIGNAL — FOR THE REPLICA-READ LOGGER.
|
||||
* `civitai_app_block_rest_approval_verdicts_total{reason="lookup_failed"}` is, and it is
|
||||
* UNTHROTTLED, so that throttle only bounds the prose. Do not add a metric there and do
|
||||
* not read a suppressed log as a suppressed verdict.
|
||||
*
|
||||
* ⚠️ THE SAME HOLDS FOR THE SECOND CONSUMER **ON REST ONLY**, AND THIS CONSTANT NOW SITS
|
||||
* ABOVE BOTH. `tunnelFailureLog`'s failures carry their own
|
||||
* `reason="tunnel_lookup_failed"` label, unthrottled like every other verdict — so on the
|
||||
* REST surface, suppressing a line loses prose and not information. (An earlier draft
|
||||
* argued the log was that leg's only signal, which is what the dedicated verdict exists to
|
||||
* make untrue.)
|
||||
*
|
||||
* 🔴 IT IS STILL TRUE ON THE BRIDGE, AND SAYING OTHERWISE WOULD BE THIS CHANGE'S OWN
|
||||
* MISTAKE ONE SURFACE WIDER. `recordBlockRestApprovalVerdict` has ONE production call
|
||||
* site, in `withBlockScope`; `assertAppBlockApproved` resolves the same verdict and
|
||||
* records NOTHING. So a tunnel failure reached through the bridge emits no counter at all
|
||||
* and its only trace is this throttled line — on a deployment that does not collect
|
||||
* container logs. The gap is PRE-EXISTING and equally true of `not_approved` (the bridge
|
||||
* has never recorded a verdict), so it is not something this change introduced and closing
|
||||
* it is a bridge-metrics change rather than a guard one — but the bridge is the
|
||||
* higher-rate surface, `pollWorkflow` being timer-driven, so **a zero on
|
||||
* `reason="tunnel_lookup_failed"` does not mean the leg is healthy.** Read it as a REST
|
||||
* signal, not a system one.
|
||||
*
|
||||
* ⚠️ THE WINDOW IS SHARED FOR CONVENIENCE, NOT BECAUSE THE RATE ARGUMENT IS THE SAME, and
|
||||
* a previous version of this line claimed it was. They are very different: the replica
|
||||
* logger's case is fleet-wide simultaneity across ALL traffic — every block REST request
|
||||
* on every pod fails at once — while the tunnel logger is reachable only on the dev +
|
||||
* real-row + NOT-approved path, i.e. the dev-tunnel owners. (⚠️ The triggering fault is a
|
||||
* sysRedis incident, which hits every pod at once too, so this is the same simultaneity
|
||||
* over a much smaller POPULATION, not a different shape — an earlier draft of this line
|
||||
* overstated the difference.) By this repo's own reasoning a population that small may not
|
||||
* need throttling at all (`block-scope.middleware` leaves its
|
||||
* `not_found` line unthrottled precisely because it "is bounded by one app's traffic
|
||||
* rather than the whole fleet's"). 60s is kept anyway because it costs nothing now that
|
||||
* the counter carries the signal, and one window is one thing to reason about — but if
|
||||
* this log ever becomes load-bearing again, that is the assumption to revisit first.
|
||||
*/
|
||||
const LOOKUP_FAILURE_LOG_WINDOW_MS = 60_000;
|
||||
let lookupFailureLastLoggedAt = 0;
|
||||
let lookupFailureDroppedSinceLastLog = 0;
|
||||
|
||||
/**
|
||||
* 🔴 ONE THROTTLE IMPLEMENTATION, SEPARATE WINDOWS PER FAILURE MODE. The logic was written
|
||||
* once and open-coded once; there are now two failure modes that need it (the replica read
|
||||
* and the dev-tunnel re-check), and giving them a SHARED window would be wrong in the
|
||||
* expensive direction: two simultaneous incidents would suppress each other, and the one
|
||||
* you did not see would be the one you most needed. Each caller gets its own closure, so
|
||||
* each logs its first occurrence immediately and each reports its own
|
||||
* `droppedSinceLastLog`.
|
||||
*/
|
||||
function makeThrottledWarn(prefix: string): { warn: (err: unknown) => void; reset: () => void } {
|
||||
let lastLoggedAt = 0;
|
||||
let droppedSinceLastLog = 0;
|
||||
return {
|
||||
warn(err: unknown): void {
|
||||
const now = Date.now();
|
||||
if (lastLoggedAt !== 0 && now - lastLoggedAt < LOOKUP_FAILURE_LOG_WINDOW_MS) {
|
||||
droppedSinceLastLog++;
|
||||
return;
|
||||
}
|
||||
const dropped = droppedSinceLastLog;
|
||||
droppedSinceLastLog = 0;
|
||||
lastLoggedAt = now;
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(
|
||||
`${prefix}: ${
|
||||
err instanceof Error ? err.message : String(err)
|
||||
} droppedSinceLastLog=${dropped} windowMs=${LOOKUP_FAILURE_LOG_WINDOW_MS}`
|
||||
);
|
||||
},
|
||||
reset(): void {
|
||||
lastLoggedAt = 0;
|
||||
droppedSinceLastLog = 0;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const lookupFailureLog = makeThrottledWarn('[block-scope] approved-status lookup failed');
|
||||
|
||||
function warnLookupFailed(err: unknown): void {
|
||||
const now = Date.now();
|
||||
if (
|
||||
lookupFailureLastLoggedAt !== 0 &&
|
||||
now - lookupFailureLastLoggedAt < LOOKUP_FAILURE_LOG_WINDOW_MS
|
||||
) {
|
||||
lookupFailureDroppedSinceLastLog++;
|
||||
return;
|
||||
}
|
||||
const droppedSinceLastLog = lookupFailureDroppedSinceLastLog;
|
||||
lookupFailureDroppedSinceLastLog = 0;
|
||||
lookupFailureLastLoggedAt = now;
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn(
|
||||
`[block-scope] approved-status lookup failed: ${
|
||||
err instanceof Error ? err.message : String(err)
|
||||
} droppedSinceLastLog=${droppedSinceLastLog} windowMs=${LOOKUP_FAILURE_LOG_WINDOW_MS}`
|
||||
);
|
||||
lookupFailureLog.warn(err);
|
||||
}
|
||||
|
||||
/** TEST-ONLY: reset the per-pod log-throttle window so tests don't share state. */
|
||||
/**
|
||||
* 🔴 THE DEV-TUNNEL LEG'S OWN LOG, AND IT EXISTS BECAUSE THE FIRST VERSION OF THAT LEG HAD
|
||||
* NONE. The `try/catch` around the tunnel re-check converts a throw into
|
||||
* `tunnel_lookup_failed` — correct as a VERDICT, and for two rounds it was `not_approved`,
|
||||
* which was wrong as OBSERVABILITY: before the wrapper existed such a throw reached
|
||||
* `resolveRestApprovalVerdict`, was logged here, and answered
|
||||
* `lookup_failed`. Swallowing it silently folded a cache incident into
|
||||
* `…verdicts_total{reason="not_approved"}` — the *same* series this change ships to be
|
||||
* watched on, and the one the predicate's docblock calls the operator's only view of the
|
||||
* 4h window closing. A sysRedis fault would then have read as the narrowing working.
|
||||
*
|
||||
* ⚠️ THE LOG IS THE SECONDARY SIGNAL, AND TWO EARLIER DRAFTS OF THIS PARAGRAPH HAD THAT
|
||||
* BACKWARDS. The first said the tunnel leg needed no verdict because "the REST mapping for
|
||||
* an unknown verdict is 503" — false, and false in the fail-open direction: REST's chain is
|
||||
* `not_approved` → 403, `lookup_failed` → 503, then an `else` asserting
|
||||
* `approval satisfies 'not_found'` that logs "SERVING (observe-only)" and falls through to
|
||||
* the handler, so its runtime default for an unrecognised verdict is to SERVE. The second
|
||||
* corrected that but still argued this log was what separated a cache incident from the
|
||||
* stale-token population — which it cannot be, because **application-container logs are
|
||||
* not collected on this deployment** (`app-block-runtime.metrics.ts` says so twice and
|
||||
* designs around it). A signal nobody can read is not a signal.
|
||||
*
|
||||
* So the leg got the verdict it needed: `tunnel_lookup_failed`, refusing identically to
|
||||
* `not_approved` on both callers but counted under its own `reason=` label. THIS log is
|
||||
* kept because it costs nothing and is genuinely useful anywhere container logs ARE
|
||||
* collected (local, and any future deployment that turns them on).
|
||||
*
|
||||
* ⚠️ "NO LONGER LOAD-BEARING" IS TRUE ON REST ONLY, and an earlier version of this line
|
||||
* said it flat — which contradicted the paragraph ninety lines above stating that a
|
||||
* tunnel failure reached through the BRIDGE emits no counter and has this line as its
|
||||
* only trace. Both cannot be true of the same logger. On REST the verdict carries the
|
||||
* signal and this is prose; on the bridge it is everything there is, on a deployment that
|
||||
* cannot read it. That asymmetry is the honest description, and it is an argument for
|
||||
* giving the bridge a verdict counter — not for trusting this log.
|
||||
*/
|
||||
const tunnelFailureLog = makeThrottledWarn('[block-scope] dev-tunnel re-check failed');
|
||||
|
||||
/** TEST-ONLY: reset the per-pod log-throttle windows so tests don't share state. */
|
||||
export function __resetApprovalLookupFailureLogThrottleForTests(): void {
|
||||
lookupFailureLastLoggedAt = 0;
|
||||
lookupFailureDroppedSinceLastLog = 0;
|
||||
lookupFailureLog.reset();
|
||||
tunnelFailureLog.reset();
|
||||
}
|
||||
|
||||
@@ -77,8 +77,10 @@ import { resolveAppBlockApprovalVerdict } from '~/server/services/blocks/block-a
|
||||
* The read itself is issued by the shared predicate rather than spelled here, which moves
|
||||
* where it lives and not what it costs. `pollWorkflow` is the shape to think about: a
|
||||
* running block polls it on a timer, so that pair is paid per poll, per open block
|
||||
* instance. A `dev` token skips the DB read (the predicate short-circuits on the
|
||||
* exemption, before the query) but still pays both Redis GETs.
|
||||
* instance. ⚠️ A `dev` token USED TO skip the DB read; as of clawgate #571 it does not —
|
||||
* that skip was the hole, not an optimisation. Only a `reviewRunForReal` token still
|
||||
* short-circuits ahead of the query. A dev token on a real, NOT-approved row
|
||||
* additionally pays a dev-tunnel lookup (two sysRedis GETs); no other token does.
|
||||
*
|
||||
* 🔴 AND THE ORDER THIS PUT THE RATE LIMITER IN. `checkBlockCatalogRateLimit` has five
|
||||
* call sites in `blocks.router.ts`, covering seven of the fifteen bridge procedures. Four
|
||||
@@ -151,20 +153,23 @@ export async function authorizeBlockBridgeToken(blockToken: string): Promise<Blo
|
||||
* The backing `app_blocks` row must still be `approved`. Resolved by the same
|
||||
* `(appId, blockId)` unique the sibling resolvers use, and from the token's claims only.
|
||||
*
|
||||
* 🔴 THE ONE EXEMPTION — a `dev` token, and it is a documented product decision, not an
|
||||
* oversight. `/api/v1/block-tokens`'s `tryDevTunnelOwnedNonApprovedMint` mints a dev
|
||||
* token carrying the app's REAL ids for an app that is deliberately NOT approved: a
|
||||
* suspended / pending / deprecated app stays runnable by its OWNER inside the owner's own
|
||||
* dev tunnel, so they can diagnose it back into review. That path is contained by its own
|
||||
* belt — ownership enforced in the query, an ACTIVE dev tunnel required, author +
|
||||
* dev-tunnel flags, self-bound `sub`, forced-SFW, dev-budget-capped, and never public.
|
||||
* Enforcing approval here would break it. The dev-token mints that have no backing row at
|
||||
* all (the pending / local-manifest / review-sandbox paths, which sign a synthetic
|
||||
* `pubreq_…` / `page_local_…` / `ephemeral-…` appBlockId) are covered by the same
|
||||
* exemption for the same reason: there is no row to be approved.
|
||||
* 🔴 THE EXEMPTION IS NO LONGER "A `dev` TOKEN", AND THIS PARAGRAPH USED TO SAY IT WAS.
|
||||
* It justified a bare `claims.dev === true` short-circuit by listing the belts
|
||||
* `tryDevTunnelOwnedNonApprovedMint` enforces — ownership in-query, an ACTIVE dev tunnel,
|
||||
* author + dev-tunnel flags, self-bound `sub`, forced-SFW, budget cap — while this guard
|
||||
* re-checked NONE of them and keyed on the signed boolean alone. Since the `dev` claim is
|
||||
* stamped by six different mint paths, that argument covered one of them and exempted all
|
||||
* six, for the 4h dev lifetime (16× the 900s default). clawgate #571.
|
||||
*
|
||||
* Revocation above is NOT exempted — every one of those mints stamps a revocable instance
|
||||
* id, so a dev token is still killable.
|
||||
* The shared predicate now re-derives the two belts that actually discriminate — the
|
||||
* subject IS the app's owner, and that owner has an ACTIVE dev tunnel for the slug — so a
|
||||
* `dev:live` token whose app was approved at mint and has since been suspended is
|
||||
* REFUSED here, while the owner-dev-tunnel path and the review sandbox keep working.
|
||||
* `resolveAppBlockApprovalVerdict`'s own docblock is the argument and the population
|
||||
* table; do not restate it here, and do not re-derive the exemption from the `dev` claim.
|
||||
*
|
||||
* Revocation above is NOT exempted, and never was — every one of those mints stamps a
|
||||
* revocable instance id, so a dev token is still killable regardless of this verdict.
|
||||
*
|
||||
* 🔴 THE LOOKUP IS SHARED WITH THE REST GATE; THE POLICY IS NOT. `resolveAppBlockApprovalVerdict`
|
||||
* (`block-approval.service.ts`) is the one place the row is read and `approved` is compared,
|
||||
@@ -178,10 +183,26 @@ export async function authorizeBlockBridgeToken(blockToken: string): Promise<Blo
|
||||
* first-party postMessage surface reached only through the host page, this is its
|
||||
* long-standing behaviour, and nothing here argued for changing it — so it did not
|
||||
* change. If you make these agree, make it a decision, not a refactor.
|
||||
* - a read that THROWS — propagates from here exactly as it always has, surfacing as
|
||||
* - a ROW read that THROWS — propagates from here exactly as it always has, surfacing as
|
||||
* the tRPC internal error. The REST gate converts it to a fail-closed 503 instead.
|
||||
* That mapping lives in `resolveRestApprovalVerdict`, which this function does not
|
||||
* call, precisely so the conversion does not reach the bridge.
|
||||
* ⚠️ SCOPED TO THE ROW READS SINCE clawgate #571, and this line used to be blanket.
|
||||
* The predicate's dev-tunnel re-check is wrapped at its own call, so a cache fault on
|
||||
* THAT leg no longer reaches here as an internal error — a bridge caller gets
|
||||
* `FORBIDDEN / 'app block is not approved'`, deliberately indistinguishable from a
|
||||
* real refusal (telling a block that the dev-tunnel cache is down would be an
|
||||
* infrastructure oracle), plus a throttled warn this path never used to emit.
|
||||
*
|
||||
* 🔴 WHAT SEPARATES THAT INCIDENT FROM A REAL REFUSAL IS THE `tunnel_lookup_failed`
|
||||
* VERDICT — NOT THE LOG, and an earlier version of this note said the log. It is not
|
||||
* readable: container logs are not collected on this deployment. But the verdict is
|
||||
* only half a signal HERE, because **this path records no counter at all** — the
|
||||
* verdict metric has a single call site, in `withBlockScope`. So on the bridge a
|
||||
* tunnel-cache fault is genuinely unobservable today. That gap predates this change
|
||||
* and is equally true of `not_approved`; closing it means giving the bridge a verdict
|
||||
* counter, which is a metrics change rather than a guard one. Recorded here so the
|
||||
* next reader does not infer from the REST series that this surface is covered.
|
||||
*/
|
||||
async function assertAppBlockApproved(claims: BlockTokenClaims): Promise<void> {
|
||||
const verdict = await resolveAppBlockApprovalVerdict(claims);
|
||||
@@ -189,7 +210,12 @@ async function assertAppBlockApproved(claims: BlockTokenClaims): Promise<void> {
|
||||
if (verdict === 'not_found') {
|
||||
throw new TRPCError({ code: 'NOT_FOUND', message: 'app block not found' });
|
||||
}
|
||||
if (verdict === 'not_approved') {
|
||||
if (verdict === 'not_approved' || verdict === 'tunnel_lookup_failed') {
|
||||
// 🔴 THE SAME REFUSAL FOR BOTH, DELIBERATELY — identical code AND identical message.
|
||||
// `tunnel_lookup_failed` is a separate verdict so the REST counter can attribute it,
|
||||
// not so the caller can tell the two apart: a block that learned "the dev-tunnel cache
|
||||
// is down" rather than "not approved" would be a state oracle on infrastructure, for
|
||||
// no benefit to it. The split is for the operator, not the bearer.
|
||||
throw new TRPCError({ code: 'FORBIDDEN', message: 'app block is not approved' });
|
||||
}
|
||||
// Compile-time exhaustiveness: a verdict added to the shared union fails to build here
|
||||
|
||||
@@ -2,8 +2,13 @@ import { dbWrite } from '~/server/db/client';
|
||||
import {
|
||||
BlockRevocation,
|
||||
isSubjectScopedInstanceId,
|
||||
subjectForUserId,
|
||||
} from '~/server/services/block-revocation.service';
|
||||
// Taken from the LEAF rather than through `block-revocation.service`'s re-export. That
|
||||
// module is wholesale-`vi.mock`ed in a dozen suites with a factory exporting only
|
||||
// `BlockRevocation`, so importing a second symbol from it resolves to `undefined` in any
|
||||
// suite that mocks it while exercising this service for real — the exact shape the leaf
|
||||
// was extracted to avoid. Inert today; this keeps it that way.
|
||||
import { subjectForUserId } from '~/server/services/block-token-subject';
|
||||
import { listActiveDevTunnelBlockIds } from '~/server/services/blocks/dev-tunnel.service';
|
||||
import { limitConcurrency } from '~/server/utils/concurrency-helpers';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user