Skip to content

Commit 45ced00

Browse files
os-muskos-muskclaude
authored
refactor(core): one shared admission tenancy-posture classification, six seams folded onto it (#17101)
* refactor(core): fold the six admission tenancy-posture classifications onto one helper The classification (branded "never registered" stays quiet, every other rejection becomes AuthzStoreUnavailableError('tenancy', err)) was hand-written at six admission seams. Any one of them degrading into a silent `catch` re-opens #13906. It is now `classifyAdmissionTenancyPosture` in @objectstack/core, with its own two-direction pins. The RESOLUTION deliberately did not move: rest-server's kernel-vs-provider branch, each seam's accessor-presence guard, and each seam's own reason why a missing async accessor stays quiet are per-seam facts a shared owner would have had to erase or carry a flag for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * chore: changeset for the admission tenancy-posture classification fold Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(core): route the helper's source pin through the shared comment mask `pnpm check:comment-mask-adoption` reds on a private regex comment stripper; the shared `scripts/js-comment-mask.mjs` answers that question once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(core): read the helper's implementation by SYMBOL SLICE, not by stripping comments `check:cross-package-test-inputs` reds on the shared comment mask's specifier: importing it makes this test's real inputs wider than its package, which needs a glob widening plus a `vitest.repo-tests.json` entry. The reading does not need comment stripping at all — the module doc is what names the forbidden symbols, and it sits ABOVE the declaration. Slicing from the declaration to end of file needs no stripper, keeps the test inside its package, and the preconditions of the slice (found, starts at the declaration, comment-free) are now themselves asserted instead of assumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * fix(spec): the retirement walk stops reading tsup's transient bundle, and a vanished path is not a crash `Test Core (1/6)` failed on this PR's head with `ENOENT ... open 'packages/spec/tsup.config.bundled_8xzodswt4ct.mjs'` raised by `compliance-families-retirement.test.ts`'s `readFileSync`. The cause is the walk's POPULATION, not this PR's diff: - `turbo.json` gives `test:repo` `dependsOn: ["^build"]` — UPSTREAM builds only, never its own package's — so `@objectstack/spec#build` runs concurrently with the walk; - tsup bundles `tsup.config.ts` to `tsup.config.bundled_<random>.mjs` beside it, loads it and deletes it. `.gitignore` already declares that class (`*.bundled_*.mjs`), but the walk is a filesystem walk, not a git walk, so it enumerated the artifact anyway; - `readdirSync` then `readFileSync` is not atomic, so the file is enumerated, deleted, and the read raises ENOENT. Reproduced deterministically on this tree: planting `packages/spec/tsup.config.bundled_REPROBE.mjs` containing a retired name makes the leg report it as an offender (exit 1) before this change, and exit 0 after — proof the artifact really was in the radius, under a filename that changes every run. Two halves, and ⛔ neither skips or re-scopes a real source: - the transient bundle is excluded by name pattern. It is a COPY of `tsup.config.ts`, which the walk still reads, so this removes no coverage; - a path that vanishes between enumeration and read is recorded and skipped — a file that no longer exists cannot be a reference that SURVIVES in the tree, which is exactly what the pin asserts. ⛔ Every non-ENOENT read fault is still re-raised. Both are pinned: a narrowness control (the pattern must not reach `tsup.config.ts`, `index.mjs` or any authored `.mjs`) and a vanished-path control with a positive read and an EISDIR leg. Ablating the pattern to `/\.mjs$/` reddens the narrowness control, so it can fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: os-musk <elon@objectstack.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 8a70e1b commit 45ced00

12 files changed

Lines changed: 742 additions & 186 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
'@objectstack/core': minor
3+
'@objectstack/rest': patch
4+
'@objectstack/cloud-connection': patch
5+
'@objectstack/plugin-sharing': patch
6+
'@objectstack/service-datasource': patch
7+
'@objectstack/service-settings': patch
8+
'@objectstack/service-storage': patch
9+
---
10+
11+
refactor(core): one `classifyAdmissionTenancyPosture`, so six admission seams cannot each get the classification wrong (#16013)
12+
13+
Six admission doors each hand-wrote the same try/catch on the `tenancy` read that
14+
feeds `resolveAuthzContext`: the registry's branded "never registered" rejection
15+
(`isServiceNotRegisteredError`, #13905) resolves quietly to `undefined` — the
16+
supported no-tenancy composition, where no posture-conditional refusal runs at
17+
all — and every other rejection becomes `AuthzStoreUnavailableError('tenancy', err)`
18+
(ADR-0112 `SERVICE_UNAVAILABLE` / 503), because the posture is an authorization
19+
INPUT and admission was therefore never DECIDED. That is #13906 decision 1
20+
option A, and it is the part nobody may get wrong: a quiet `catch` at any one of
21+
the six re-opens the defect, where a failure reads as "this check does not apply"
22+
and an ex-member's org-stamped API key is admitted.
23+
24+
Nothing is broken today — every copy was correct — so this removes a standing
25+
hazard rather than fixing a defect. **No admission verdict changes**, on any
26+
wiring: the classification is byte-for-byte the decision the six copies made,
27+
now made once.
28+
29+
- **`@objectstack/core` gains `classifyAdmissionTenancyPosture`** (and the
30+
`TenancyServiceResolver` type), exported from the package index beside
31+
`effectiveTenancyPosture`. It takes a THUNK and owns the classification only.
32+
The thunk is not a style choice: the REJECTION is what gets classified, so the
33+
resolution has to happen inside the helper's `try` — a caller that awaited the
34+
service first would need a `catch` of its own, which is the thing being
35+
deleted.
36+
- **The RESOLUTION deliberately did not move.** `rest-server.ts` branches on
37+
kernel-vs-provider, and asking twice would let a provider bound to the local
38+
kernel answer for a request that resolved to another environment; four seams
39+
read `ctx.getKernel()`; `service-storage` reads an already-normalised gate
40+
registry; and each seam's reason why a MISSING async accessor must stay quiet
41+
is its own argument (the storage door's is its declared degrade-to-ungated
42+
contract, the others' is the `KernelBase`/`LiteKernel` host shape). A helper
43+
that also owned how the service is reached would be wrong for one of them or
44+
grow a flag per seam — the copies again, with an extra step. Every one of
45+
those reasons stays written at its seam.
46+
- **Folded**: `packages/rest/src/rest-server.ts` (both wirings),
47+
`packages/cloud-connection/src/marketplace-install-local-plugin.ts`,
48+
`packages/plugins/plugin-sharing/src/sharing-plugin.ts`,
49+
`packages/services/service-datasource/src/admin-routes.ts`,
50+
`packages/services/service-settings/src/settings-service-plugin.ts`,
51+
`packages/services/service-storage/src/storage-service-plugin.ts`.
52+
- **Pinned where the decision now lives**:
53+
`packages/core/src/security/admission-tenancy-posture.test.ts` drives both
54+
rejections at the production seam — a real `ObjectKernel` that never
55+
registered `tenancy`, and one whose `tenancy` factory throws — each beside the
56+
brand predicate's own answer on that same rejection, so "the outage throws" is
57+
distinguishable from a helper that throws at everything. It also holds the
58+
constraint mechanically: the helper's source may not name an accessor, a
59+
kernel or a plugin context, and it takes exactly one parameter.

packages/cloud-connection/src/marketplace-install-local-plugin.ts

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,12 @@ import type { Plugin, PluginContext } from '@objectstack/core';
4949
import {
5050
resolveAuthzContext,
5151
isAuthzStoreUnavailableError,
52-
// [#15353] The three symbols the ADMISSION seam's posture derivation needs
53-
// — the same set `packages/rest`'s repaired seam imports, for the same
54-
// reason. `effectiveTenancyPosture` reads the posture IN FORCE off the
55-
// kernel's `tenancy` service; the other two are decision-1-option-A's
56-
// classification (#13906): never-registered is branded and quiet, every
57-
// other rejection is the outage it is.
58-
effectiveTenancyPosture,
59-
isServiceNotRegisteredError,
60-
AuthzStoreUnavailableError,
52+
// [#16013] The ADMISSION seam's posture derivation, in ONE call — the same
53+
// symbol `packages/rest`'s seam now imports, for the same reason. It reads
54+
// the posture IN FORCE off the kernel's `tenancy` service and carries
55+
// decision-1-option-A's classification (#13906): never-registered is
56+
// branded and quiet, every other rejection is the outage it is.
57+
classifyAdmissionTenancyPosture,
6158
type TenancyPostureSource,
6259
} from '@objectstack/core';
6360
import {
@@ -1673,15 +1670,19 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
16731670
* ⛔ Do not wire either of them into this seam, and ⛔ do not add a
16741671
* `catch { undefined }` here.
16751672
*
1676-
* ## The classification, decision 1 option A (#13906)
1673+
* ## The classification, decision 1 option A (#13906) — no longer here
16771674
*
1678-
* - **Never registered** ⇒ branded (`isServiceNotRegisteredError`), quiet
1679-
* `undefined`. A lean embedding with no `plugin-auth` is a SUPPORTED
1680-
* composition, and behaviour there is exactly what it was.
1681-
* - **Registered and unable to answer** ⇒ `AuthzStoreUnavailableError`
1682-
* (ADR-0112 `SERVICE_UNAVAILABLE` / 503). Admission was never DECIDED, so
1683-
* it must not be answered. {@link resolveInstallPrincipal}'s `catch`
1684-
* already re-raises this brand rather than collapsing it to `null` (401).
1675+
* [#16013] `classifyAdmissionTenancyPosture` (`@objectstack/core`) owns the
1676+
* branded/unbranded decision for every admission seam: never registered ⇒
1677+
* quiet `undefined`; every other rejection ⇒ `AuthzStoreUnavailableError`
1678+
* (ADR-0112 `SERVICE_UNAVAILABLE` / 503), because admission was never
1679+
* DECIDED and must not be answered.
1680+
*
1681+
* ⚠️ What that means AT THIS DOOR is still this door's own: a lean
1682+
* embedding with no `plugin-auth` is a SUPPORTED composition and behaviour
1683+
* there is exactly what it was, and on the loud arm
1684+
* {@link resolveInstallPrincipal}'s `catch` already re-raises this brand
1685+
* rather than collapsing it to `null` (401).
16851686
*
16861687
* ⚠️ The brand exists only on the ASYNC resolution path: `PluginContext.getService`
16871688
* throws two UNBRANDED plain `Error`s (`… not found` and `… is async - use
@@ -1703,16 +1704,9 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
17031704
| { getServiceAsync?: <T>(name: string, scopeId?: string) => Promise<T> }
17041705
| undefined;
17051706
if (!kernel || typeof kernel.getServiceAsync !== 'function') return undefined;
1706-
try {
1707-
return effectiveTenancyPosture(
1708-
await kernel.getServiceAsync<TenancyPostureSource>('tenancy'),
1709-
);
1710-
} catch (err) {
1711-
if (!isServiceNotRegisteredError(err)) {
1712-
throw new AuthzStoreUnavailableError('tenancy', err);
1713-
}
1714-
return undefined;
1715-
}
1707+
return classifyAdmissionTenancyPosture(() =>
1708+
kernel.getServiceAsync!<TenancyPostureSource>('tenancy'),
1709+
);
17161710
};
17171711

17181712
private resolveInstallPrincipal = async (

0 commit comments

Comments
 (0)