Skip to content

Commit 7cb922e

Browse files
authored
chore(spec): scan platform-object definitions for permissive empty states (#3966)
Follow-up to #3896/#3945. That gate required any "empty = permissive" statement in the spec to be classified on purpose — and its scan scope had a hole big enough to miss the bug it was built for. The sentence that shipped #3896, "leave empty to share every record", was the description of sys_sharing_rule.criteria_json, which lives in plugin-sharing. The gate could not see its own crime scene. Now scans **/*.object.ts anywhere under packages/ — plugins, platform-objects, metadata-core, and the create-objectstack templates, since a starter file is the highest-leverage thing a model copies from. 214 → 290 files. It immediately found a real one. sys_user_permission_set.organization_id declares "NULL = applies in every org context": a user↔permission-set grant with no org scope applies everywhere. Deliberate and load-bearing rather than an oversight — ADR-0095 D3 / ADR-0068 D2 derive the platform_admin posture from an UNSCOPED admin_full_access grant specifically, and an org-scoped grant of the same set must not confer it. The empty state is not merely wider; it is the distinguishing input to the highest privilege in the system. Registered `open` with that rationale and both enforcement sites cited. Three fixes the new surface forced: repudiated prose no longer fires (#3929's own "Deliberately NOT ..." comment was being flagged, with the escape kept narrow so a genuine claim carrying an unrelated negation is still caught); the owning property is resolved by nesting rather than a name list (skipping `description` answered `required`, the sibling above it — indentation is what separates a field from its config); and the property-search window is per-surface, since widening it globally would mis-attribute .zod.ts narrative to a distant property. Evidence is now parsed with the liveness ledger's own checkEvidence, so prose and multiple paths work — the README already promised that, and a raw-path existsSync quietly did not. 6 new unit tests (32 total). spec: 263 files / 6862 tests green, tsc clean, both gates green. No runtime behaviour changes.
1 parent 9e01213 commit 7cb922e

6 files changed

Lines changed: 344 additions & 36 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
chore(spec): the empty-state gate now scans platform-object definitions, where #3896 actually happened
6+
7+
#3945 added a gate requiring any *"empty = permissive"* statement in the spec to
8+
be classified on purpose. It scanned `packages/spec/src/**/*.zod.ts` — and that
9+
scope had a hole big enough to miss the bug it was built for.
10+
11+
The sentence that shipped #3896, *"leave empty to share every record"*, was the
12+
`description` of `sys_sharing_rule.criteria_json`, which lives in
13+
**`plugin-sharing`**. The gate could not see its own crime scene.
14+
15+
**Now scans `**/*.object.ts` anywhere under `packages/`** — plugins,
16+
`platform-objects`, `metadata-core`, and the `create-objectstack` templates
17+
(a starter file is the highest-leverage thing a model copies from). 214 → 290
18+
files.
19+
20+
**It immediately found a real one.** `sys_user_permission_set.organization_id`
21+
declares *"NULL = applies in every org context"*: a user↔permission-set grant with
22+
no org scope applies everywhere. That is deliberate and load-bearing rather than
23+
an oversight — ADR-0095 D3 / ADR-0068 D2 derive the `platform_admin` posture from
24+
an **unscoped** `admin_full_access` grant specifically, and an org-scoped grant of
25+
the same set must not confer it. So the empty state is not merely wider, it is the
26+
distinguishing input to the highest privilege in the system. Registered `open`
27+
with that rationale and both enforcement sites cited, which is the point: the
28+
answer now lives somewhere other than a maintainer's memory.
29+
30+
Three fixes the new surface forced, each a case of the gate being wrong in a way
31+
that mattered:
32+
33+
- **Repudiated prose no longer fires.** #3929's own comment on `criteria_json`
34+
reads *Deliberately NOT "leave empty to share everything"* — the gate flagged the
35+
sentence recording why the gate exists. Negation is now handled for the
36+
imperative form as well as the token form (`deny-all`), and the escape is
37+
deliberately narrow: the negator must be attached to the phrase, not merely
38+
present in the line, because a false negative here is a missed over-share.
39+
- **The owning property is found by nesting, not by a name list.** A field's prose
40+
sits in a nested key, so the first attempt answered `description` for every
41+
platform-object hit; skipping doc slots then answered `required`, the sibling
42+
above it. What separates a field from its own config is indentation, so the
43+
resolver now takes the nearest key at a shallower indent.
44+
- **The property-search window is per-surface.** A platform-object `description:`
45+
can sit 15+ lines below its field name; a `.zod.ts` statement sits beside its
46+
property. Widening globally would let `.zod.ts` narrative be mis-attributed to a
47+
distant property — turning a correct non-failing note into a wrong failure — so
48+
`.object.ts` gets a wider window and the schema surface keeps its tight one.
49+
50+
Also makes evidence resolution honest: entries are now parsed with the liveness
51+
ledger's own `checkEvidence`, so prose around the paths works, several paths can
52+
be cited, and another repo's path (`objectui: …`) is recorded without being
53+
resolved here. The README already promised evidence resolved "like the ledger's";
54+
a single raw-path `existsSync` quietly did not.
55+
56+
6 new unit tests (32 total). No runtime behaviour changes.

packages/spec/liveness/README.md

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -337,13 +337,15 @@ Three properties, same syntactic shape (an optional list or predicate that
337337
| Property | Empty means |
338338
|---|---|
339339
| `object.apiMethods` | `undefined` = unrestricted, **`[]` = deny-all** |
340-
| `plugin-runtime.allowedSources` | was *"empty = all allowed"* — corrected |
340+
| `sys_user_permission_set.organization_id` | NULL = the grant applies in **every** org — and is what derives `platform_admin` |
341+
| `plugin-runtime.allowedSources` | was *"empty = all allowed"* — schema since removed (#3950) |
341342
| sharing `condition` | nothing is shared (#3929) |
342343

343344
Nothing marked which was which. A maintainer knows by memory; a model authoring
344345
metadata cannot, and guessing wrong is silent and permissive. So the gate scans
345-
the schema surface for statements declaring an empty state to be permissive, and
346-
requires each to be classified in `../scripts/liveness/empty-state-registry.mts`:
346+
the authorable surface for statements declaring an empty state to be permissive,
347+
and requires each to be classified in
348+
`../scripts/liveness/empty-state-registry.mts`:
347349

348350
| `semantics` | Meaning |
349351
|---|---|
@@ -373,6 +375,41 @@ check is a check nobody reads. A statement that resolves to no property is
373375
narrative — a file header explaining a past bug — and is reported as a
374376
non-failing note.
375377

378+
### What it scans, and why it is not just `packages/spec`
379+
380+
Two surfaces:
381+
382+
- `packages/spec/src/**/*.zod.ts` — the schema surface.
383+
- `**/*.object.ts` anywhere under `packages/` — platform-object definitions, in
384+
plugins, `platform-objects`, `metadata-core`, and the `create-objectstack`
385+
templates (a starter file is the highest-leverage thing a model copies from).
386+
387+
**The second one is the point.** The sentence that shipped #3896*"leave empty
388+
to share every record"* — was the `description` of
389+
`sys_sharing_rule.criteria_json`, which lives in `plugin-sharing`. A gate scoped
390+
to `packages/spec` could not see the crime scene. Extending the scan immediately
391+
surfaced one unclassified access-control default-open that no sweep of the schema
392+
surface would ever have reached (`sys_user_permission_set.organization_id`).
393+
394+
Two things the two surfaces do NOT share, both learned by getting them wrong:
395+
396+
- **The property-search window.** A `.zod.ts` statement sits on or beside its
397+
property; a platform-object field is a nested call whose `description:` can be
398+
15+ lines below the name. The window is therefore per-surface — widening it
399+
globally would let `.zod.ts` narrative be mis-attributed to a distant property,
400+
turning a correct note into a wrong failure.
401+
- **How the owning property is found.** Not by a list of key names. Skipping
402+
`description:` is not enough — the next key up from it is `required:`, equally
403+
not the field. What separates a field from its own config is *nesting*, so the
404+
resolver takes the nearest key at a **shallower indent**.
405+
406+
Finally, a match whose permissive claim is **negated or explicitly disowned** is
407+
dropped, in both directions: `[]` = `deny-all` is the opposite of the hazard, and
408+
so is #3929's own comment saying *Deliberately NOT "leave empty to share
409+
everything"*. The escape is narrow on purpose — the negator has to be attached to
410+
the phrase, not merely nearby, because a false negative here is a missed
411+
over-share.
412+
376413
## Files & usage
377414

378415
- `<type>.json` — the ledger for a governed metadata type.

packages/spec/scripts/liveness/check-empty-state.mts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,39 @@ import { checkEmptyState } from './empty-state.mts';
1717
const here = dirname(fileURLToPath(import.meta.url));
1818
const specRoot = resolve(here, '../..'); // packages/spec
1919
const repoRoot = resolve(specRoot, '../..');
20-
const srcRoot = join(specRoot, 'src');
21-
22-
/** The authorable spec surface: the schema files an author (or a model) reads. */
23-
function collectSchemaFiles(dir: string, out: string[] = []): string[] {
20+
const packagesRoot = resolve(repoRoot, 'packages');
21+
22+
// Build output, dependencies and caches are copies — scanning them would report
23+
// the same statement several times and flag paths nobody edits.
24+
const SKIP_DIRS = new Set(['node_modules', 'dist', 'build', '.turbo', 'coverage', '.git']);
25+
26+
/**
27+
* The authorable surface: everything an author — very often a model — reads as
28+
* the contract.
29+
*
30+
* TWO kinds of file, and the second is the reason this gate exists at all:
31+
*
32+
* - `packages/spec/src/**\/*.zod.ts` — the schema surface.
33+
* - `**\/*.object.ts` — platform-object definitions, wherever they live (plugins,
34+
* `platform-objects`, `metadata-core`, the `create-objectstack` templates).
35+
* The sentence that shipped #3896 — *"leave empty to share every record"* — was
36+
* the `description` of `sys_sharing_rule.criteria_json`, which lives in
37+
* `plugin-sharing`. A gate scoped to `packages/spec` could not see the crime
38+
* scene. Templates are included deliberately: a starter file is the highest-
39+
* leverage thing a model copies from.
40+
*/
41+
function collectAuthorableFiles(dir: string, out: string[] = []): string[] {
2442
for (const entry of readdirSync(dir, { withFileTypes: true })) {
25-
const full = join(dir, entry.name);
2643
if (entry.isDirectory()) {
27-
collectSchemaFiles(full, out);
28-
} else if (entry.name.endsWith('.zod.ts') && !entry.name.endsWith('.test.ts')) {
29-
out.push(full);
44+
if (SKIP_DIRS.has(entry.name)) continue;
45+
collectAuthorableFiles(join(dir, entry.name), out);
46+
continue;
3047
}
48+
if (entry.name.endsWith('.test.ts')) continue;
49+
const full = join(dir, entry.name);
50+
const rel = relative(repoRoot, full);
51+
if (entry.name.endsWith('.object.ts')) out.push(full);
52+
else if (entry.name.endsWith('.zod.ts') && rel.startsWith(join('packages', 'spec', 'src'))) out.push(full);
3153
}
3254
return out;
3355
}
@@ -36,7 +58,7 @@ const args = process.argv.slice(2);
3658
const asJson = args.includes('--json');
3759
const dump = args.includes('--dump');
3860

39-
const files = collectSchemaFiles(srcRoot).sort();
61+
const files = collectAuthorableFiles(packagesRoot).sort();
4062
const sources = new Map<string, string>(
4163
files.map((f) => [relative(repoRoot, f), readFileSync(f, 'utf8')]),
4264
);

packages/spec/scripts/liveness/empty-state-registry.mts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,16 @@ export const EMPTY_STATE_REGISTRY: EmptyStateEntry[] = [
107107
evidence: 'packages/spec/src/security/sharing.zod.ts',
108108
},
109109

110+
{
111+
file: 'packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts',
112+
property: 'organization_id',
113+
semantics: 'open',
114+
rationale:
115+
"A NULL organization scope on a user↔permission-set grant means the grant is UNSCOPED — it applies in every org context. Deliberate and load-bearing rather than an oversight: ADR-0095 D3 / ADR-0068 D2 DERIVE the platform_admin posture from an unscoped `admin_full_access` user grant specifically, and an org-SCOPED grant of the same set must not confer it. So the empty state is not merely wider, it is the distinguishing input to the highest privilege in the system — which is exactly why it belongs on this list rather than being left to memory. `explain-engine.ts` recomputes the identical predicate on purpose so the explain panel's posture cannot sit higher than enforcement's.",
116+
evidence:
117+
'packages/core/src/security/resolve-authz-context.ts (resolveAuthzContext.hasPlatformAdminGrant — the single source of truth) and packages/plugins/plugin-security/src/explain-engine.ts (the identical predicate, replicated so the panel cannot overstate)',
118+
},
119+
110120
// ---- Scope selectors ---------------------------------------------------
111121

112122
{

0 commit comments

Comments
 (0)