Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/25561-analytics-admission-bridge-503.md
Original file line number Diff line number Diff line change
@@ -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

<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition, stored shape or exported declaration is added, removed, renamed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What changes is the code and status one analytics refusal answers when the `security` service is missing or cannot be used (403 PERMISSION_DENIED to 503 SERVICE_UNAVAILABLE); every request that was refused is still refused and every request that was served is still served. The remedy is on the consumer's side, reading the code, or on the deployment's side, registering a security service; never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->

**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`.
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,20 @@ 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.
const { service, reads, lines } = await bootKernel(undefined);

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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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([]);
Expand All @@ -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(
Expand All @@ -189,16 +195,16 @@ 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);
expect(result.rows).toEqual([{ cnt: 24 }]);
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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
});
Expand Down
14 changes: 8 additions & 6 deletions packages/services/service-analytics/src/analytics-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand Down Expand Up @@ -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<string>,
Expand Down
32 changes: 21 additions & 11 deletions packages/services/service-analytics/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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') {
Expand Down
Loading
Loading