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
44 changes: 44 additions & 0 deletions .changeset/unknown-key-strictness-step2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
"@objectstack/spec": major
---

feat(spec)!: reject unknown keys on RLS policies, sharing rules, and positions (#4001 step 2)

Second click of the unknown-key strictness ratchet (first: flow + permission,
#4071), extending `.strict()` + the `strictUnknownKeyError` fixable-error
factory to the remaining small security-class authoring surfaces, per
`docs/audits/2026-07-unknown-key-strictness-ledger.md`:

- **`security/rls.zod.ts`** — `RowLevelSecurityPolicySchema` is `.strict()`.
A silently dropped key on an RLS policy meant a row-level restriction the
author wrote was never compiled into the filter. The runtime shapes
(`RLSUserContextSchema`, `RLSEvaluationResultSchema`) stay tolerant. The
retired `priority` key keeps its existing tombstone.
- **`security/sharing.zod.ts`** — the sharing-rule surface is `.strict()`
(base + criteria extension + the `sharedWith` recipient shape). A silently
dropped key meant a share the author intended was never materialised.
- **`identity/position.zod.ts`** — `PositionSchema` is `.strict()`, and gains
the author-facing `protection` block plus the ADR-0010 runtime protection
envelope (`_lock`, `_packageId`, `_provenance`, …) — closing the sibling
gap the #4071 ledger flagged: `applyProtection` stamps every registered
metadata type, and position was the last one whose schema could not
represent the stamp.

**Migration.** Any key these schemas now reject was previously stripped and
had **no runtime effect** — removing or renaming it never changes behavior.
The error carries the fix; FROM → TO mappings baked in include:

- RLS policy: `roles`/`role` → `positions` (ADR-0090 D3 rename),
`withCheck` → `check` (the PostgreSQL spelling), `condition`/`filter`/`where`
→ `using`. `priority` stays a tombstone (#3896: OR-combined policies have no
precedence to order — delete the key).
- Sharing rule: `criteria` → `condition` (the persisted row spells the
compiled predicate `criteria_json`; the authored key is the CEL
`condition`), `access`/`level` → `accessLevel`,
`recipient`/`shareWith`/`sharedTo` → `sharedWith`, `enabled` → `active`;
recipient `id`/`target` → `value`. `ownedBy` carries the removed
owner-type-rule prescription (only `criteria` rules are authorable).
- Position: `title` → `label`; `permissionSets` / `users` are runtime
bindings (`sys_position_permission_set` / `sys_user_position`), never
authored on the position; `parent` is rejected with the flatness rule
(ADR-0090 D3 — hierarchy is the business-unit tree, not a position tree).
8 changes: 8 additions & 0 deletions content/docs/references/identity/position.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ const result = Position.parse(data);
| **label** | `string` | ✅ | Display label (e.g. VP of Sales) |
| **description** | `string` | optional | |
| **delegatable** | `boolean` | ✅ | ADR-0091 D3: holders may self-service delegate this position, time-boxed (default false). |
| **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this position. |
| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). |
| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. |
| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). |
| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). |
| **_packageId** | `string` | optional | Owning package machine id. |
| **_packageVersion** | `string` | optional | Owning package version. |
| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. |


---
Expand Down
16 changes: 15 additions & 1 deletion content/docs/releases/v17.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ model), while `url` platform-wide means an HTTP endpoint to call (`http` node,
webhooks). The singular `input` on `map` / `subflow` / `connector_action` is
those nodes' own canonical key and is untouched.

### Flow and permission schemas reject unknown keys (#4001)
### Flow, permission, RLS, sharing and position schemas reject unknown keys (#4001)

Zod's default is `.strip`: a key a schema does not declare is silently
discarded and the instance keeps parsing. On an authorable surface that is the
Expand All @@ -327,6 +327,20 @@ schema to the two highest-risk authorable surfaces, per the triage in
`FlowVariableSchema`. A node's `config` stays an **open** record: it is
per-node-type, owned by the executor's `configSchema` and the conversion
layer (see the alias table above).
- **RLS policies** — `RowLevelSecurityPolicySchema`. A silently dropped key
here meant a row-level restriction the author wrote was never compiled into
the filter (the pre-ADR-0090 D3 vocabulary → `positions`,
`withCheck` → `check`, `condition`/`filter`/`where` → `using`). The runtime
evaluation shapes stay tolerant.
- **Sharing rules** — the rule, its criteria extension, and the `sharedWith`
recipient (`criteria` → `condition`, `access` → `accessLevel`,
`recipient` → `sharedWith`; `ownedBy` carries the removed owner-type-rule
prescription).
- **Positions** — `PositionSchema`, including the guidance that
`permissionSets`/`users` are runtime bindings and `parent` has no meaning on
a deliberately flat position (ADR-0090 D3). Position also gains the
`protection` block and ADR-0010 runtime envelope every sibling registered
type already declared.

Every rejection is written to be self-fixing: it names the offending key and,
where recognisable, the canonical spelling (`steps` → `nodes`, edge
Expand Down
35 changes: 23 additions & 12 deletions docs/audits/2026-07-unknown-key-strictness-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,19 @@ strict gate could prove it. Note the asymmetry in the two schema gaps — both
were *inverse* drift (runtime writes a key the spec cannot express), which the
liveness ledger's per-property direction cannot see.

**Known sibling gap (follow-up, not this step):** `identity/position.zod.ts`
— the other registered security type — also omits `MetadataProtectionFields`
while `applyProtection` stamps it. Harmless today because the schema is not
strict (the keys are silently stripped, as permission's were), but it must be
declared before `position` joins the ratchet.
**Known sibling gap — CLOSED in step 2:** `identity/position.zod.ts` — the
other registered security type — also omitted `MetadataProtectionFields` while
`applyProtection` stamps it. Declared (with the author-facing `protection`
block) when `position` joined the ratchet.

4. **`position.test.ts` asserted a fictional hierarchy** (found in step 2 when
`PositionSchema` went strict): the pre-ADR-0090 test "should accept position
with parent" — plus four "real-world hierarchy" examples — authored a
`parent` key on positions. It only ever "passed" because `.strip` ate the
key; no position tree exists (ADR-0090 D3 finalized flatness; hierarchy is
the business-unit tree). The tests were codifying the strip-era fiction as
expected behavior. Rewritten: `parent` is now asserted to be *rejected* with
the flatness guidance.

## File-level triage — the five authorable directories

Expand Down Expand Up @@ -168,8 +176,8 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts).
|---|---|---|---|
| `explain.zod.ts` | 11 | wire | permission-explain responses — never strict |
| `permission.zod.ts` | 4 | authorable | **strict as of #4001**; `EffectiveObjectPermissionSchema` explicitly `.strip()`s (wire) |
| `rls.zod.ts` | 3 | authorable | **next candidate** a stripped RLS key is a silent policy hole |
| `sharing.zod.ts` | 2 | authorable | **next candidate** — same class |
| `rls.zod.ts` | 3 | authorable | **`RowLevelSecurityPolicySchema` strict as of #4001 step 2** (a stripped RLS key is a silent policy hole); `RLSUserContextSchema` / `RLSEvaluationResultSchema` are runtime shapes — stay tolerant |
| `sharing.zod.ts` | 2 | authorable | **strict as of #4001 step 2** — rule + recipient shapes; strictness and the error map ride the base into the criteria extension |

### `studio/` — 27 sites

Expand All @@ -189,20 +197,23 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts).
| `cloud/` | 83 | wire | multi-tenant runtime |
| `ai/` | 75 | mixed | agent/tool/skill definitions authored (partially strict already); model/provider payloads wire |
| `integration/` | 64 | wire | connector payloads — upstream adds fields freely |
| `identity/` | 34 | mixed | position/user shapes authored; auth payloads wire |
| `identity/` | 34 | mixed | position/user shapes authored (`PositionSchema` **strict as of #4001 step 2**, with the ADR-0010 envelope declared); auth payloads wire |
| `shared/` | 25 | n/a | utilities and building blocks; strictness decided at the consuming schema |
| `qa/` | 6 | n/a | test fixtures |

## Next steps (verify-then-enforce, one shape at a time)

1. `ui/app.zod.ts` — `AppSchema` + navigation union (highest-traffic remaining
authorable type; needs union-error design so the strict error is readable).
2. `security/rls.zod.ts` + `security/sharing.zod.ts` — small, security-class.
3. `automation/approval.zod.ts` — new v17 authoring surface, tighten while young.
4. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config.
5. Promote this ledger to a machine-checked gate (pattern of
2. `automation/approval.zod.ts` — new v17 authoring surface, tighten while young.
3. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config.
4. Promote this ledger to a machine-checked gate (pattern of
`packages/spec/liveness/` + `check:liveness`) once enough of the surface is
classified that the table above is enforceable rather than descriptive.

Done in step 2 (this PR): `security/rls.zod.ts` + `security/sharing.zod.ts`
strict; `PositionSchema` strict with the protection envelope declared (closing
the known sibling gap below).

Long tail stays gated on a verification pass per shape — never a one-shot
"make all ~453 sites strict" (ADR-0054 ratchet; #4001's own recommendation).
8 changes: 8 additions & 0 deletions packages/spec/authorable-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -3876,10 +3876,18 @@
"identity/Organization:name",
"identity/Organization:slug",
"identity/Organization:updatedAt",
"identity/Position:_lock",
"identity/Position:_lockDocsUrl",
"identity/Position:_lockReason",
"identity/Position:_lockSource",
"identity/Position:_packageId",
"identity/Position:_packageVersion",
"identity/Position:_provenance",
"identity/Position:delegatable",
"identity/Position:description",
"identity/Position:label",
"identity/Position:name",
"identity/Position:protection",
"identity/SCIMAddress:country",
"identity/SCIMAddress:formatted",
"identity/SCIMAddress:locality",
Expand Down
90 changes: 62 additions & 28 deletions packages/spec/src/identity/position.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,24 @@ describe('PositionSchema', () => {
expect(() => PositionSchema.parse(position)).not.toThrow();
});

it('should accept position with parent', () => {
const position: Position = {
it('rejects the retired `parent` hierarchy key (positions are flat, ADR-0090 D3)', () => {
// Pre-ADR-0090 this test asserted `parent` parsed — which only "worked"
// because zod's default `.strip` silently ate the key; no position tree
// ever existed. Since #4001 the fiction is a loud error instead.
const result = PositionSchema.safeParse({
name: 'vp_sales',
label: 'VP of Sales',
parent: 'ceo',
};

expect(() => PositionSchema.parse(position)).not.toThrow();
});
expect(result.success).toBe(false);
expect(
result.error!.issues.find((i) => i.code === 'unrecognized_keys')!.message,
).toContain('business-unit');
});
});

describe('Real-World Position Examples', () => {
it('should accept complete sales organization hierarchy', () => {
it('should accept a complete sales organization (flat — ADR-0090 D3)', () => {
const positions: Position[] = [
{
name: 'ceo',
Expand All @@ -83,31 +88,26 @@ describe('PositionSchema', () => {
{
name: 'vp_sales',
label: 'VP of Sales',
parent: 'ceo',
description: 'Leads entire sales organization',
},
{
name: 'regional_sales_director',
label: 'Regional Sales Director',
parent: 'vp_sales',
description: 'Manages sales for a specific region',
},
{
name: 'sales_manager',
label: 'Sales Manager',
parent: 'regional_sales_director',
description: 'Manages a team of sales representatives',
},
{
name: 'senior_sales_rep',
label: 'Senior Sales Representative',
parent: 'sales_manager',
description: 'Senior member of sales team',
},
{
name: 'sales_rep',
label: 'Sales Representative',
parent: 'sales_manager',
description: 'Individual contributor in sales',
},
];
Expand All @@ -117,7 +117,7 @@ describe('PositionSchema', () => {
});
});

it('should accept service organization hierarchy', () => {
it('should accept a service organization (flat)', () => {
const positions: Position[] = [
{
name: 'vp_customer_success',
Expand All @@ -126,17 +126,14 @@ describe('PositionSchema', () => {
{
name: 'support_manager',
label: 'Support Manager',
parent: 'vp_customer_success',
},
{
name: 'senior_support_agent',
label: 'Senior Support Agent',
parent: 'support_manager',
},
{
name: 'support_agent',
label: 'Support Agent',
parent: 'support_manager',
},
];

Expand All @@ -145,7 +142,7 @@ describe('PositionSchema', () => {
});
});

it('should accept product organization hierarchy', () => {
it('should accept a product organization (flat)', () => {
const positions: Position[] = [
{
name: 'cto',
Expand All @@ -154,28 +151,23 @@ describe('PositionSchema', () => {
{
name: 'vp_engineering',
label: 'VP of Engineering',
parent: 'cto',
},
{
name: 'engineering_manager',
label: 'Engineering Manager',
parent: 'vp_engineering',
description: 'Manages engineering team',
},
{
name: 'tech_lead',
label: 'Technical Lead',
parent: 'engineering_manager',
},
{
name: 'senior_engineer',
label: 'Senior Software Engineer',
parent: 'engineering_manager',
},
{
name: 'engineer',
label: 'Software Engineer',
parent: 'engineering_manager',
},
];

Expand All @@ -184,7 +176,7 @@ describe('PositionSchema', () => {
});
});

it('should accept matrix organization with multiple reporting lines', () => {
it('should accept a matrix organization (reporting lines live on sys_user.manager_id, not positions)', () => {
const positions: Position[] = [
{
name: 'ceo',
Expand All @@ -193,27 +185,22 @@ describe('PositionSchema', () => {
{
name: 'regional_vp_americas',
label: 'Regional VP - Americas',
parent: 'ceo',
},
{
name: 'regional_vp_emea',
label: 'Regional VP - EMEA',
parent: 'ceo',
},
{
name: 'regional_vp_apac',
label: 'Regional VP - APAC',
parent: 'ceo',
},
{
name: 'country_manager_us',
label: 'Country Manager - US',
parent: 'regional_vp_americas',
},
{
name: 'country_manager_uk',
label: 'Country Manager - UK',
parent: 'regional_vp_emea',
},
];

Expand All @@ -231,7 +218,6 @@ describe('PositionSchema', () => {
{
name: 'team_member',
label: 'Team Member',
parent: 'founder',
},
];

Expand All @@ -241,3 +227,51 @@ describe('PositionSchema', () => {
});
});
});

// #4001 step 2 — PositionSchema joins the strict ratchet, and gains the
// ADR-0010 protection envelope the #4071 ledger flagged as the known sibling
// gap (applyProtection stamps EVERY registered metadata type; position could
// not represent the stamp).
describe('unknown keys are rejected, not stripped (#4001)', () => {
const unknownKeyIssue = (value: unknown) => {
const result = PositionSchema.safeParse(value);
expect(result.success).toBe(false);
return result.error!.issues.find((i) => i.code === 'unrecognized_keys');
};

it('rejects an undeclared key instead of silently dropping it', () => {
expect(unknownKeyIssue({ name: 'p', label: 'P', notAKey: 1 })!.message)
.toContain('`notAKey`');
});

it('points permissionSets/users/parent at the runtime binding or flatness rule', () => {
expect(unknownKeyIssue({ name: 'p', label: 'P', permissionSets: [] })!.message)
.toContain('sys_position_permission_set');
expect(unknownKeyIssue({ name: 'p', label: 'P', users: [] })!.message)
.toContain('sys_user_position');
expect(unknownKeyIssue({ name: 'p', label: 'P', parent: 'boss' })!.message)
.toContain('FLAT');
});

it('round-trips the ADR-0010 runtime protection envelope', () => {
const parsed = PositionSchema.parse({
name: 'auditor', label: 'Auditor',
_packageId: 'com.showcase', _provenance: 'package', _lock: 'full',
});
expect(parsed._packageId).toBe('com.showcase');
expect(parsed._lock).toBe('full');
});

it('accepts every key the schema declares (guards POSITION_KEYS drift)', () => {
const probes: Record<string, unknown> = {
description: 'd', delegatable: true, protection: { lock: 'none' },
};
for (const [key, value] of Object.entries(probes)) {
const result = PositionSchema.safeParse({ name: 'p', label: 'P', [key]: value });
const unknown = result.success
? undefined
: result.error.issues.find((i) => i.code === 'unrecognized_keys');
expect(unknown, `\`${key}\` should be a declared Position key`).toBeUndefined();
}
});
});
Loading
Loading