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
56 changes: 56 additions & 0 deletions .changeset/empty-state-gate-object-surface.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
"@objectstack/spec": patch
---

chore(spec): the empty-state gate now scans platform-object definitions, where #3896 actually happened

#3945 added a gate requiring any *"empty = permissive"* statement in the spec to
be classified on purpose. It scanned `packages/spec/src/**/*.zod.ts` — and that
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
(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. That is 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. So 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, which is the point: the
answer now lives somewhere other than a maintainer's memory.

Three fixes the new surface forced, each a case of the gate being wrong in a way
that mattered:

- **Repudiated prose no longer fires.** #3929's own comment on `criteria_json`
reads *Deliberately NOT "leave empty to share everything"* — the gate flagged the
sentence recording why the gate exists. Negation is now handled for the
imperative form as well as the token form (`deny-all`), and the escape is
deliberately narrow: the negator must be attached to the phrase, not merely
present in the line, because a false negative here is a missed over-share.
- **The owning property is found by nesting, not by a name list.** A field's prose
sits in a nested key, so the first attempt answered `description` for every
platform-object hit; skipping doc slots then answered `required`, the sibling
above it. What separates a field from its own config is indentation, so the
resolver now takes the nearest key at a shallower indent.
- **The property-search window is per-surface.** A platform-object `description:`
can sit 15+ lines below its field name; a `.zod.ts` statement sits beside its
property. Widening globally would let `.zod.ts` narrative be mis-attributed to a
distant property — turning a correct non-failing note into a wrong failure — so
`.object.ts` gets a wider window and the schema surface keeps its tight one.

Also makes evidence resolution honest: entries are now parsed with the liveness
ledger's own `checkEvidence`, so prose around the paths works, several paths can
be cited, and another repo's path (`objectui: …`) is recorded without being
resolved here. The README already promised evidence resolved "like the ledger's";
a single raw-path `existsSync` quietly did not.

6 new unit tests (32 total). No runtime behaviour changes.
43 changes: 40 additions & 3 deletions packages/spec/liveness/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,13 +337,15 @@ Three properties, same syntactic shape (an optional list or predicate that
| Property | Empty means |
|---|---|
| `object.apiMethods` | `undefined` = unrestricted, **`[]` = deny-all** |
| `plugin-runtime.allowedSources` | was *"empty = all allowed"* — corrected |
| `sys_user_permission_set.organization_id` | NULL = the grant applies in **every** org — and is what derives `platform_admin` |
| `plugin-runtime.allowedSources` | was *"empty = all allowed"* — schema since removed (#3950) |
| sharing `condition` | nothing is shared (#3929) |

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

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

### What it scans, and why it is not just `packages/spec`

Two surfaces:

- `packages/spec/src/**/*.zod.ts` — the schema surface.
- `**/*.object.ts` anywhere under `packages/` — platform-object definitions, in
plugins, `platform-objects`, `metadata-core`, and the `create-objectstack`
templates (a starter file is the highest-leverage thing a model copies from).

**The second one is the point.** 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`. A gate scoped
to `packages/spec` could not see the crime scene. Extending the scan immediately
surfaced one unclassified access-control default-open that no sweep of the schema
surface would ever have reached (`sys_user_permission_set.organization_id`).

Two things the two surfaces do NOT share, both learned by getting them wrong:

- **The property-search window.** A `.zod.ts` statement sits on or beside its
property; a platform-object field is a nested call whose `description:` can be
15+ lines below the name. The window is therefore per-surface — widening it
globally would let `.zod.ts` narrative be mis-attributed to a distant property,
turning a correct note into a wrong failure.
- **How the owning property is found.** Not by a list of key names. Skipping
`description:` is not enough — the next key up from it is `required:`, equally
not the field. What separates a field from its own config is *nesting*, so the
resolver takes the nearest key at a **shallower indent**.

Finally, a match whose permissive claim is **negated or explicitly disowned** is
dropped, in both directions: `[]` = `deny-all` is the opposite of the hazard, and
so is #3929's own comment saying *Deliberately NOT "leave empty to share
everything"*. The escape is narrow on purpose — the negator has to be attached to
the phrase, not merely nearby, because a false negative here is a missed
over-share.

## Files & usage

- `<type>.json` — the ledger for a governed metadata type.
Expand Down
40 changes: 31 additions & 9 deletions packages/spec/scripts/liveness/check-empty-state.mts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,39 @@ import { checkEmptyState } from './empty-state.mts';
const here = dirname(fileURLToPath(import.meta.url));
const specRoot = resolve(here, '../..'); // packages/spec
const repoRoot = resolve(specRoot, '../..');
const srcRoot = join(specRoot, 'src');

/** The authorable spec surface: the schema files an author (or a model) reads. */
function collectSchemaFiles(dir: string, out: string[] = []): string[] {
const packagesRoot = resolve(repoRoot, 'packages');

// Build output, dependencies and caches are copies — scanning them would report
// the same statement several times and flag paths nobody edits.
const SKIP_DIRS = new Set(['node_modules', 'dist', 'build', '.turbo', 'coverage', '.git']);

/**
* The authorable surface: everything an author — very often a model — reads as
* the contract.
*
* TWO kinds of file, and the second is the reason this gate exists at all:
*
* - `packages/spec/src/**\/*.zod.ts` — the schema surface.
* - `**\/*.object.ts` — platform-object definitions, wherever they live (plugins,
* `platform-objects`, `metadata-core`, the `create-objectstack` templates).
* 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`. A gate scoped to `packages/spec` could not see the crime
* scene. Templates are included deliberately: a starter file is the highest-
* leverage thing a model copies from.
*/
function collectAuthorableFiles(dir: string, out: string[] = []): string[] {
for (const entry of readdirSync(dir, { withFileTypes: true })) {
const full = join(dir, entry.name);
if (entry.isDirectory()) {
collectSchemaFiles(full, out);
} else if (entry.name.endsWith('.zod.ts') && !entry.name.endsWith('.test.ts')) {
out.push(full);
if (SKIP_DIRS.has(entry.name)) continue;
collectAuthorableFiles(join(dir, entry.name), out);
continue;
}
if (entry.name.endsWith('.test.ts')) continue;
const full = join(dir, entry.name);
const rel = relative(repoRoot, full);
if (entry.name.endsWith('.object.ts')) out.push(full);
else if (entry.name.endsWith('.zod.ts') && rel.startsWith(join('packages', 'spec', 'src'))) out.push(full);
}
return out;
}
Expand All @@ -36,7 +58,7 @@ const args = process.argv.slice(2);
const asJson = args.includes('--json');
const dump = args.includes('--dump');

const files = collectSchemaFiles(srcRoot).sort();
const files = collectAuthorableFiles(packagesRoot).sort();
const sources = new Map<string, string>(
files.map((f) => [relative(repoRoot, f), readFileSync(f, 'utf8')]),
);
Expand Down
10 changes: 10 additions & 0 deletions packages/spec/scripts/liveness/empty-state-registry.mts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@ export const EMPTY_STATE_REGISTRY: EmptyStateEntry[] = [
evidence: 'packages/spec/src/security/sharing.zod.ts',
},

{
file: 'packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts',
property: 'organization_id',
semantics: 'open',
rationale:
"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.",
evidence:
'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)',
},

// ---- Scope selectors ---------------------------------------------------

{
Expand Down
Loading
Loading