diff --git a/.changeset/25561-analytics-admission-bridge-503.md b/.changeset/25561-analytics-admission-bridge-503.md new file mode 100644 index 00000000000..f5d2444681f --- /dev/null +++ b/.changeset/25561-analytics-admission-bridge-503.md @@ -0,0 +1,18 @@ +--- +'@objectstack/service-analytics': patch +--- + +fix(service-analytics)!: the analytics admission bridge answers `503 SERVICE_UNAVAILABLE`, not a denial's `403 PERMISSION_DENIED`, when the `security` service is missing or cannot be used + +`AnalyticsServicePlugin` bridges the object-level read admission to the `security` service. When that service cannot be used, the bridge refuses the query, fail-closed. That covers three cases: resolving the service throws, the service exposes neither `canReadObject` nor `explain`, or no `security` service was ever registered (`ObjectKernel` and `LiteKernel` throw on a lookup for a service nobody registered). Until now the refusal answered `403 PERMISSION_DENIED`, the same answer as a caller who may not read the object. The bridge reached no verdict, so the refusal now answers `503 SERVICE_UNAVAILABLE` naming the object, like the other analytics read gates that cannot decide. The cause stays in the server log. Fail-closed is unchanged: nothing is read, and nothing that was refused is now served. A real denial (`canReadObject` or `explain` answering false) still answers `403 PERMISSION_DENIED`, byte for byte. The bridge's `error` log line no longer says "A security service is wired on this deployment". That was untrue when no security service was ever registered. The line now says the `security` service is missing or cannot be used. The row-level read scope bridge's `error` line said the same untrue sentence and is corrected the same way; what that bridge answers does not change. + +Clause-②: no +Metadata-protocol: none + + + +**BREAKING** (the wire answer of one fault refusal changes; no request that was served is refused and none that was refused is served), shipped as `patch` because the metadata protocol does not move. + +- **Before:** with no `admitObjectRead` supplied, an analytics query on a deployment whose `security` service is broken, or was never registered, was refused `403 PERMISSION_DENIED`. That was the same code and status as a caller who genuinely may not read the object. +- **Now:** the same refusal answers `503 SERVICE_UNAVAILABLE` (`status: 503`, `object` naming the object). On `POST /api/v1/analytics/dataset/query` the body is `{"code":"SERVICE_UNAVAILABLE","error":"Internal server error"}`. Real denials still answer `403 PERMISSION_DENIED`, unchanged. +- **Fix:** a client that treated every analytics `403` as "no access" should treat `503 SERVICE_UNAVAILABLE` as a server-side fault and check the server log. A deployment that wants analytics to answer registers a `security` service (for example `plugin-security`), or its host supplies its own `admitObjectRead`. diff --git a/packages/services/service-analytics/src/__tests__/admission-absence-report.test.ts b/packages/services/service-analytics/src/__tests__/admission-absence-report.test.ts index 5f180ca714f..5acf1eb3cf4 100644 --- a/packages/services/service-analytics/src/__tests__/admission-absence-report.test.ts +++ b/packages/services/service-analytics/src/__tests__/admission-absence-report.test.ts @@ -157,7 +157,7 @@ describe('admission absence report — a real kernel that never registers a secu it('logs no init-time WARN, and absence stays loud at query time with the outcome unchanged', async () => { // The in-repo kernels answer a missing service by THROWING // ("[Kernel] Service 'security' not found"), which this bridge classifies - // as a resolution that threw: it denies, fail-closed, and says so at + // as a resolution that threw: it refuses, fail-closed, and says so at // `error` on every query. That outcome is what this boot produced before // the init-time WARN moved, and it is not this file's to change; what is // pinned is that dropping the init-time WARN left this case loud. @@ -165,9 +165,12 @@ describe('admission absence report — a real kernel that never registers a secu expect(about(lines.warn)).toEqual([]); + // [#25561] No verdict was reached, so the refusal is the fault answer + // (`SERVICE_UNAVAILABLE` / 503, naming the object), not a denial's 403. await expect(runProbe(service)).rejects.toMatchObject({ - code: 'PERMISSION_DENIED', - status: 403, + code: 'SERVICE_UNAVAILABLE', + status: 503, + object: 'employer_member', }); expect(reads).toEqual([]); expect(about(lines.error).join('\n')).toMatch( diff --git a/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts b/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts index c2836672f04..463baa8a756 100644 --- a/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts +++ b/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts @@ -20,20 +20,23 @@ * - the service resolved but exposes NEITHER `canReadObject` NOR `explain` * (UNUSABLE) — it exists and cannot answer. * - * Both UNUSABLE corners DENY, fail-closed, before anything is read, and both say - * why at `error`. For a wired-but-broken provider, `/data`'s middleware does not - * fall open either, so admitting here would reopen exactly the divergence - * between the two doors that this gate closes — and would do it silently. + * Both UNUSABLE corners REFUSE, fail-closed, before anything is read, and both + * say why at `error`. They reached no verdict, so the refusal answers + * `503 SERVICE_UNAVAILABLE`, not a denial's `403 PERMISSION_DENIED` (#25561). + * For a wired-but-broken provider, `/data`'s middleware does not fall open + * either, so admitting here would reopen exactly the divergence between the + * two doors that this gate closes — and would do it silently. * - * ## The declaration: no security service registered ⇒ analytics DENIES + * ## The declaration: no security service registered ⇒ analytics REFUSES * * [#22235] A deployment that registers no security service gets its analytics * read queries REFUSED, fail-closed: the kernel's lookup throws, the bridge - * takes UNUSABLE, the caller gets `PERMISSION_DENIED` / 403 naming the object, - * and the operator gets an `error` line naming the object and the failed - * lookup. That is the declared answer, not an accident of how the lookup fails, - * and it holds even though `/data` carries no object-level gate on such a - * deployment: a loud deny is preferred over a silent admit. A composition that + * takes UNUSABLE, the caller gets `SERVICE_UNAVAILABLE` / 503 naming the object + * (#25561: the code moved from 403, the refusal did not), and the operator + * gets an `error` line naming the object and the failed lookup. That is the + * declared answer, not an accident of how the lookup fails, and it holds even + * though `/data` carries no object-level gate on such a deployment: a loud + * refusal is preferred over a silent admit. A composition that * wants analytics to answer registers a security service, or its host supplies * its own `admitObjectRead`. The last block of this file pins it on both * in-repo kernels. @@ -151,13 +154,15 @@ const runProbe = (service: AnalyticsService) => describe('analytics admission bridge — resolving the "security" service', () => { // ── The two corners that used to admit silently ──────────────────────────── - it('DENIES when resolving the "security" service THROWS', async () => { + it('REFUSES 503 when resolving the "security" service THROWS', async () => { const boom = () => { throw new Error('security service is initialising'); }; const { service, reads, error } = await bootAnalytics(boom); + // [#25561] No verdict was reached, so the refusal is a fault, not a denial. await expect(runProbe(service)).rejects.toMatchObject({ - code: 'PERMISSION_DENIED', - status: 403, + code: 'SERVICE_UNAVAILABLE', + status: 503, + object: 'employer_member', }); // The refusal has to happen BEFORE the statement runs, or it is not a gate. expect(reads).toEqual([]); @@ -168,15 +173,16 @@ describe('analytics admission bridge — resolving the "security" service', () = ); }); - it('DENIES when the "security" service exposes neither canReadObject nor explain', async () => { + it('REFUSES 503 when the "security" service exposes neither canReadObject nor explain', async () => { // A registered object that is not the contract it claims to be — // `explain` is NON-optional on `ISecurityService`, so a conforming // provider never lands here. const { service, reads, error } = await bootAnalytics(() => ({ getReadFilter: () => undefined })); await expect(runProbe(service)).rejects.toMatchObject({ - code: 'PERMISSION_DENIED', - status: 403, + code: 'SERVICE_UNAVAILABLE', + status: 503, + object: 'employer_member', }); expect(reads).toEqual([]); expect(error.mock.calls.map((c) => String(c[0])).join('\n')).toMatch( @@ -189,8 +195,8 @@ describe('analytics admission bridge — resolving the "security" service', () = it('ADMITS when the context answers the "security" lookup with nothing (ABSENT)', async () => { // The ABSENT branch: this double answers the lookup with nothing, which no // in-repo kernel does — they throw on a miss (the last block of this file). - // Kept as the negative control for the two deny cases above; whether this - // branch should deny too is a separate question, not settled here. + // Kept as the negative control for the two refusal cases above; whether + // this branch should refuse too is a separate question, not settled here. const { service, reads } = await bootAnalytics(undefined); const result = await runProbe(service); @@ -198,7 +204,7 @@ describe('analytics admission bridge — resolving the "security" service', () = expect(reads).toHaveLength(1); }); - // ── The two working spellings, so the deny cases cannot pass by refusing all ─ + // ── The two working spellings, so the refusals cannot pass by refusing all ── it('asks canReadObject when the service has it, and serves an ADMITTED caller', async () => { const canReadObject = vi.fn(() => true); @@ -238,7 +244,7 @@ describe('analytics admission bridge — resolving the "security" service', () = }); }); -// ── The declaration, on the in-repo kernels: no security service ⇒ DENY ────── +// ── The declaration, on the in-repo kernels: no security service ⇒ REFUSE ──── /** * Boots a REAL kernel with a `data` provider and this plugin, and NO security @@ -271,17 +277,19 @@ describe.each([ ['ObjectKernel', () => new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false })], ['LiteKernel', () => new LiteKernel({ logger: { level: 'silent' } })], ] as const)('analytics admission bridge — a real %s that never registers a "security" service', (_name, makeKernel) => { - it('DENIES the dataset query fail-closed, before any read, with an error located on the object and the lookup', async () => { + it('REFUSES the dataset query 503, fail-closed, before any read, with an error located on the object and the lookup', async () => { const { kernel, service, reads, errors, warns } = await bootKernelWithoutSecurity(makeKernel()); // The mechanism the declaration rests on: this kernel answers a lookup for // a never-registered name by THROWING, never with nothing. expect(() => kernel.getService('security')).toThrow(/'security'/); - // The caller's refusal: the declared envelope, naming the object. + // The caller's refusal: the declared envelope, naming the object. [#25561] + // A service that is "not loaded" is the spec's SERVICE_UNAVAILABLE, the + // same answer a registered-but-broken one gets. await expect(runProbe(service)).rejects.toMatchObject({ - code: 'PERMISSION_DENIED', - status: 403, + code: 'SERVICE_UNAVAILABLE', + status: 503, object: 'employer_member', }); // Refused BEFORE the statement ran. diff --git a/packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts index b4c83ae28de..649a341e2f1 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts @@ -217,15 +217,18 @@ describe('analytics row-scope bridge — resolving the "security" service', () = it('refuses a throwing resolver with BOTH auto-bridges live (no door falls open)', async () => { // With no `admitObjectRead` override the object-level bridge (commit 041d9fdc6) - // answers first, with `PERMISSION_DENIED`. Pinned so the file-level - // property — a broken security service serves no analytics rows through - // EITHER door — cannot regress from the other side. + // answers first. It reached no verdict, so it answers the fault refusal, + // `SERVICE_UNAVAILABLE` / 503 naming the object, not a denial's 403 + // (#25561). Pinned so the file-level property — a broken security service + // serves no analytics rows through EITHER door — cannot regress from the + // other side. const boom = () => { throw new Error('security service is initialising'); }; const { service, reads } = await bootAnalytics(boom); await expect(runProbe(service)).rejects.toMatchObject({ - code: 'PERMISSION_DENIED', - status: 403, + code: 'SERVICE_UNAVAILABLE', + status: 503, + object: 'employer_member', }); expect(reads).toEqual([]); }); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index a1493498a56..f2f29c7b8bc 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -851,12 +851,13 @@ export interface AnalyticsServiceConfig { * construction rather than by maintenance. * * MAY be async. `false` refuses the query with `PERMISSION_DENIED` / 403; a - * THROW also refuses (fail-closed). When the hook is absent entirely no + * THROW also refuses (fail-closed), answered `SERVICE_UNAVAILABLE` / 503 + * because no verdict was reached. When the hook is absent entirely no * object-level gate applies — a host that constructs the service without * one. That is not a deployment with no security service: the plugin always - * wires this hook, and on the in-repo kernels its bridge DENIES when no - * `security` service was ever registered, because the lookup throws - * (`plugin.ts`). + * wires this hook, and on the in-repo kernels its bridge REFUSES + * (`SERVICE_UNAVAILABLE` / 503) when no `security` service was ever + * registered, because the lookup throws (`plugin.ts`). */ admitObjectRead?: ObjectReadAdmissionProvider; /** @@ -1978,8 +1979,9 @@ export class AnalyticsService implements IAnalyticsService { * A no-op when no provider is wired — a host that constructs this service * without `admitObjectRead`. `AnalyticsServicePlugin` always wires one, so a * deployment with no security service is NOT this case: on the in-repo - * kernels the bridge's lookup throws and the provider denies, fail-closed - * (`plugin.ts`, `admission-bridge-resolution.test.ts`). + * kernels the bridge's lookup throws and the provider refuses, fail-closed, + * `SERVICE_UNAVAILABLE` / 503 (`plugin.ts`, + * `admission-bridge-resolution.test.ts`). */ private async assertReadAdmitted( objects: Iterable, diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index ac44380aac7..83715b24c04 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -17,6 +17,7 @@ import { assertReadScopePlaceholdersResolvable, } from './read-scope-sql.js'; import { readScopeUnresolvedError } from './read-scope-refusal.js'; +import { readAdmissionUnresolvedError } from './read-admission.js'; // [#16206] The narrowing from a driver's FOUR-name `dialectName` to the THREE // this package's config hook declares — see the bridge below. import { asAcceptedSqlDialect, type AcceptedSqlDialect } from './text-match-sql.js'; @@ -611,7 +612,7 @@ export class AnalyticsServicePlugin implements Plugin { // `getService` answers by throwing. A failed security lookup is a // refusal everywhere else in this stack, including the object-level // bridge below, which used to be spelled exactly like this one and - // now denies. + // now refuses (`503 SERVICE_UNAVAILABLE`). return { kind: 'unusable', why: @@ -653,8 +654,8 @@ export class AnalyticsServicePlugin implements Plugin { ctx.logger.error( `[Analytics] row-level read scope could not be resolved for "${object}" — ` + `refusing the query (fail-closed): ${resolved.why}. ` + - 'A security service is wired on this deployment, so analytics must not fall ' + - 'open and serve rows with no row-level policy applied.', + 'Analytics does not fall open and serve rows with no row-level policy applied ' + + 'when the "security" service is missing or cannot be used.', ); // [commit 54b3d1d4a] Declared, not bare. `resolveReadScopes` replaces this // error with its own on the dataset path, but this provider is read @@ -712,15 +713,20 @@ export class AnalyticsServicePlugin implements Plugin { // wired-but-broken provider `/data`'s middleware does NOT fall // open either, so admitting here would reopen the exact // divergence between the two doors that this gate closes, and - // silently. It DENIES, fail-closed, and says why at `error`, - // naming the object. + // silently. It REFUSES, fail-closed, and says why at `error`, + // naming the object. It reached no verdict, so it THROWS and + // the gate answers `503 SERVICE_UNAVAILABLE`, not the 403 of + // a denial (#25561). // USABLE — ask it (below). // // [#22235] The declaration (the maintainer's ruling): a deployment that // registers no security service is REFUSED at this door, fail-closed — the - // in-repo kernels throw on the miss, so it lands in UNUSABLE. `/data` - // carries no object-level gate on such a deployment, so the two doors do - // NOT agree there: a loud deny is preferred over a silent admit. A + // in-repo kernels throw on the miss, so it lands in UNUSABLE. [#25561] The + // refusal answers `503 SERVICE_UNAVAILABLE` (the service is "not loaded"), + // the same as a registered service that is broken: the code moved from + // 403, the fail-closed refusal did not. `/data` carries no object-level + // gate on such a deployment, so the two doors do NOT agree there: a loud + // refusal is preferred over a silent admit. A // composition that wants analytics to answer registers a security service, // or its host supplies its own `admitObjectRead`. // `admission-bridge-resolution.test.ts` pins it on both in-repo kernels. @@ -818,10 +824,14 @@ export class AnalyticsServicePlugin implements Plugin { ctx.logger.error( `[Analytics] object-level read admission could not be resolved for "${object}" — ` + `denying the query (fail-closed): ${resolved.why}. ` + - 'A security service is wired on this deployment, so analytics must not fall open: ' + - 'GET /data/' + object + ' does not.', + 'The "security" service is missing or cannot be used, so no verdict was reached; ' + + 'analytics refuses rather than fall open.', ); - return false; + // [#25561] A refusal that reached no verdict does not answer as + // one: THROW, so the gate answers `503 SERVICE_UNAVAILABLE`, never + // the `403 PERMISSION_DENIED` a real denial gets. Declared, not bare, + // for the reason the row-scope bridge above gives. + throw readAdmissionUnresolvedError(object); } const svc = resolved.svc; if (typeof svc.canReadObject === 'function') { diff --git a/packages/services/service-analytics/src/read-admission.ts b/packages/services/service-analytics/src/read-admission.ts index 323b9767fd2..e84082e9fb6 100644 --- a/packages/services/service-analytics/src/read-admission.ts +++ b/packages/services/service-analytics/src/read-admission.ts @@ -57,15 +57,18 @@ * it does not answer as one: `503 SERVICE_UNAVAILABLE` * ({@link readAdmissionUnresolvedError}), the cause in the server log; * - a `security` service that is wired but cannot be used — resolving it - * throws, or it exposes neither `canReadObject` nor `explain` — denies at + * throws, or it exposes neither `canReadObject` nor `explain` — refuses at * the bridge, because `/data`'s middleware does not fall open in those - * states either (`plugin.ts`); - * - [#22235] a deployment that registers NO security service denies at the + * states either (`plugin.ts`). The bridge reached no verdict, so it throws + * and the refusal answers `503 SERVICE_UNAVAILABLE` (#25561); + * - [#22235] a deployment that registers NO security service refuses at the * bridge too, by declaration: the in-repo kernels (`ObjectKernel`, * `LiteKernel`) throw on a `security` service nothing ever registered, so - * the bridge takes its UNUSABLE branch and refuses with a located error. + * the bridge takes its UNUSABLE branch and refuses with a located error, + * `503 SERVICE_UNAVAILABLE` like the broken service above (the service is + * "not loaded"; #25561). * `/data` carries no object-level gate on such a deployment, so there the - * two doors do NOT agree: a loud deny is preferred over a silent admit. A + * two doors do NOT agree: a loud refusal is preferred over a silent admit. A * composition that wants analytics to answer registers a security service, * or its host supplies its own `admitObjectRead`; * - a context that answers the `security` lookup with NOTHING (ABSENT) is a diff --git a/scripts/analytics-reconcile/boot.ts b/scripts/analytics-reconcile/boot.ts index 496911d995d..05c36f806e6 100644 --- a/scripts/analytics-reconcile/boot.ts +++ b/scripts/analytics-reconcile/boot.ts @@ -22,10 +22,11 @@ import { resolveDateMacros } from './macros.js'; * The `security` service this reconciliation runs under: registered * EXPLICITLY, and OPEN on purpose. * - * [#22235] The analytics read bridges DENY, fail-closed, on a kernel that + * [#22235] The analytics read bridges REFUSE, fail-closed, on a kernel that * registers no security service: the kernel's lookup throws on the miss, the * bridge reads that as a security service it cannot use, and every dataset - * query is refused `PERMISSION_DENIED` — which this runner would report as a + * query is refused `SERVICE_UNAVAILABLE` / 503 (#25561; it was + * `PERMISSION_DENIED` / 403 before) — which this runner would report as a * mismatch on every widget. That is the declared behaviour, so a composition * that wants analytics to answer registers a security service. *