Skip to content

Commit f270d45

Browse files
committed
test(lint): pin the option visibleWhen member verdict at build and at the save door (#22274)
Adds the refused/accepted pins, the positive control at a formula field, the buildScope-derived allowlist parity test, and the changeset. Claude-Session: https://claude.ai/code/session_01LAi5BVvQNiYzepSAcsoFLK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 69cb2dd commit f270d45

3 files changed

Lines changed: 288 additions & 1 deletion

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
fix(lint)!: `os build` and the object save door refuse a select option's `visibleWhen` that reads a member of `ctx` or `os` the server's option check never binds, such as `os.org.id`, `os.env` or `ctx.locale` (#22274)
6+
7+
Clause-②: no (narrowing: a select option's `visibleWhen` that reads an unbound member of a bound root is refused at build and at the object save door)
8+
9+
A select option's `visibleWhen` is a gate the server enforces on write. The option check binds `record`, `previous` and the acting user, as `current_user` and its ADR-0068 aliases `user`, `ctx.user` and `os.user`. Under `ctx` and `os` it binds the `user` member and nothing else: it passes no organization and no environment. The build already refused a root the option check does not bind, such as `parent`, but it judged `ctx` and `os` as whole roots. So an option predicate that read `os.org.id != ''`, `os.env == 'prod'` or `ctx.locale == 'en'` passed `os build` and the object save door with no finding. On every write that picked the option the predicate then faulted (`No such key: org`, `env` or `locale`), the server logged "the option's gate was NOT enforced on this write", and the value was admitted.
10+
11+
The build's expression rule (`validateStackExpressions`) now judges the members of `ctx` and `os` in an option's `visibleWhen`, in the same verdict that judges its roots. A member other than `user` is refused at `error` and located at the option (`object 'NAME' · field 'FIELD' option 'VALUE' visibleWhen`). The message names the member, says that the option check binds only the `user` member under that root, and gives the remedy. Every spelling of the read is judged the same: `os.org`, `os.?org`, `os['org']` and `has(os.org)`. The object save door runs the same pass, so its verdict is the build's finding: the same rule id (`expression-invalid`), location, message and hint.
12+
13+
**BREAKING — what moves for consumers.**
14+
15+
- `os build`, `os validate` and `os lint` refuse an option `visibleWhen` that reads a member of `ctx` or `os` other than `user`, such as `os.org.id`, `os.org.tier`, `os.env` or `ctx.locale`.
16+
- An object write in publish mode that carries such an option answered 200. It now answers `422 INVALID_METADATA`, with an `expression-invalid` issue located at that option. This covers `PUT /api/v1/meta/object/:name` (and `saveMetaItem` in publish mode), the promotion of a draft (`POST /api/v1/meta/object/:name/publish`, `publishMetaItem`), and a package draft publish (`publishPackageDrafts`).
17+
18+
**Remedy.**
19+
20+
- For the caller's organization, compare `current_user.organizationId`, which the option check does bind: it holds the acting user's organization id, or `null` when the caller acts outside an organization. `os.org.id != ''` becomes `current_user.organizationId != null`. No other organization fact, such as its tier, is available to an option predicate; read a column the object declares instead.
21+
- `os.env`, `ctx.locale` and any other member: rewrite the predicate against `record.FIELD`, `previous.FIELD`, or the acting user as `current_user`.
22+
- Saving the object as a draft (`mode: 'draft'`) is still allowed, because drafts are never gated; publishing that draft is judged.
23+
24+
**Unchanged.**
25+
26+
- The server's option check is unchanged. It binds what it bound before, and an option predicate that faults is still logged and admitted. If the runtime comes to bind a member such as `os.org` for an option, this refusal is lifted for that member in the same change.
27+
- The acting user is still accepted under all four ADR-0068 spellings (`current_user`, `user`, `ctx.user`, `os.user`), and so are its fields (`current_user.positions`, `ctx.user.id`) and a grant check such as `current_user.can('OBJECT', 'edit')`.
28+
- The same members are still accepted where they are bound, such as `os.org.id` in a `formula` field's `expression`. The refusal is the option slot's alone.
29+
- A computed key such as `os[name]` names no member, so it is not judged.
30+
- Stored rows are not migrated, and they are not refused on read. An object stored before this change keeps loading until it is next saved, and that save is judged.
31+
- `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns a refusal into a logged write.
32+
- Measured before crossing: the objects this repository ships carry 5 option predicates, all on `showcase_cascade`, which read `record` (four) and `current_user` (one). None reads `ctx` or `os`. That is over the 119 objects from its `*.object.ts` files and the two `app-multi-package` sub-stacks, and over the example stacks as `defineStack` composes them (33 objects). They have 0 refusals at the build and at the door, before this change and after it.
33+
- No public export or signature moves. `validateStackExpressions(stack)` keeps its signature, and no registry entry changes.
34+
35+
<!-- adr-0087: not-required (no-migration-prescription) a refusal at `os build` and at the object save door of a select option's visibleWhen predicate that reads a member of ctx or os the server's option check does not bind: no authorable key, spelling, export or stored shape moves, and no stored row is read, rewritten or converted. A stored object whose option predicate is refused keeps loading until it is next saved, and the repair is the author's rewrite of the predicate against what the option check binds, which no ledger entry can derive. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this verdict (not already-registered); and the change is a build and door verdict, not a declaration (not runtime-interface-only or type-surface-only). -->

‎packages/lint/src/validate-expressions.test.ts‎

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { describe, it, expect } from 'vitest';
55
// a copy of it: "a future `SCOPE_ROOTS` member is covered for free" is the whole
66
// argument for the allowlist, and a hand-copied list would go green on exactly
77
// the root the rule never saw.
8-
import { SCOPE_ROOTS } from '@objectstack/formula';
8+
import { SCOPE_ROOTS, buildScope, ExpressionEngine } from '@objectstack/formula';
99
import { EVALUATED_EXPRESSION_SOURCE_REQUIRED, ExpressionInputSchema, ObjectStackSchema } from '@objectstack/spec';
1010
import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec/data';
1111
import { SharingRuleSchema } from '@objectstack/spec/security';
@@ -2088,6 +2088,143 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
20882088
});
20892089
});
20902090

2091+
/**
2092+
* ── The members of `ctx` / `os` the option check never fills (#22274) ───
2093+
*
2094+
* `ctx` and `os` pass the root verdict above, but the option check fills
2095+
* each with its `user` member only: `evaluateOptionVisibility` hands the
2096+
* evaluator `{ record, previous, user, permissions }`, and `buildScope`
2097+
* mounts `os.org` / `os.env` only from an `org` / `env` in that context.
2098+
* Measured through the built `evaluateValidationRules` with an
2099+
* authenticated caller, `os.org.id`, `os.env` and `ctx.locale` each fault
2100+
* (`No such key`) and the value is admitted, so the build refuses them.
2101+
*/
2102+
describe('a per-option `visibleWhen` member of `ctx` / `os` the option check does not bind is refused (#22274)', () => {
2103+
const detail = (visibleWhen: unknown, extra: Record<string, unknown> = {}) => ({
2104+
objects: [
2105+
{
2106+
name: 'fx_line',
2107+
fields: {
2108+
x: { type: 'text' },
2109+
locale: { type: 'text' },
2110+
tier: {
2111+
type: 'select',
2112+
options: [{ label: 'Standard', value: 'standard' }, { label: 'Gold', value: 'gold', visibleWhen }],
2113+
},
2114+
...extra,
2115+
},
2116+
},
2117+
],
2118+
});
2119+
const WHERE = "object 'fx_line' · field 'tier' option 'gold' visibleWhen";
2120+
/**
2121+
* The context the option check hands the evaluator, mirrored from its one
2122+
* call (`evaluateOptionVisibility` in ObjectQL's `rule-validator.ts`): the
2123+
* merged record, `previous`, the acting user as the engine builds it (with
2124+
* the caller's organization id) and the permission map. Nothing else.
2125+
*/
2126+
const OPTION_CHECK_CONTEXT = {
2127+
record: { x: 'a' },
2128+
previous: { x: 'a' },
2129+
user: { id: 'u1', positions: ['member'], organizationId: 'org_1' },
2130+
permissions: {},
2131+
};
2132+
2133+
it.each([
2134+
["os.org.id != ''", '`os.org`'],
2135+
["os.env == 'prod'", '`os.env`'],
2136+
["ctx.locale == 'en'", '`ctx.locale`'],
2137+
])('⭐ refuses %s at error, located at the option, naming the member and what is bound', (body, path) => {
2138+
const issues = validateStackExpressions(detail(body));
2139+
expect(issues, JSON.stringify(issues, null, 2)).toHaveLength(1);
2140+
expect(issues[0]).toMatchObject({ where: WHERE, severity: 'error', source: body });
2141+
expect(issues[0]!.message).toContain(`option 'gold' on field 'tier' reads ${path}`);
2142+
expect(issues[0]!.message).toContain('the `user` member and nothing else');
2143+
});
2144+
2145+
it('⭐ the `os.org` refusal names the bound replacement, and that replacement passes and evaluates', () => {
2146+
expect(validateStackExpressions(detail("os.org.id != ''"))[0]!.message).toContain('`current_user.organizationId`');
2147+
const body = "current_user.organizationId == 'org_1'";
2148+
expect(validateStackExpressions(detail(body))).toEqual([]);
2149+
expect(ExpressionEngine.evaluate({ dialect: 'cel', source: body }, OPTION_CHECK_CONTEXT)).toEqual({ ok: true, value: true });
2150+
});
2151+
2152+
it('⭐ CONTROL — the acting user under every ADR-0068 spelling, `record`, `previous` and `can` pass', () => {
2153+
for (const body of [
2154+
"current_user.id != ''",
2155+
"os.user.id != ''",
2156+
"'org_admin' in ctx.user.positions",
2157+
"user.id != ''",
2158+
"record.x == 'a'",
2159+
"previous.x == 'a'",
2160+
"current_user.can('fx_line', 'edit')",
2161+
// A `record` member merely spelled like a refused one.
2162+
"record.locale == 'en'",
2163+
]) {
2164+
expect(validateStackExpressions(detail(body)), body).toEqual([]);
2165+
}
2166+
});
2167+
2168+
it('⭐ POSITIVE CONTROL — the same `os.org.id` on a `formula` field, which binds `os.org`, is not refused', () => {
2169+
const atFormula = detail("record.x == 'a'", { in_org: { type: 'formula', expression: "os.org.id != ''" } });
2170+
expect(validateStackExpressions(atFormula)).toEqual([]);
2171+
});
2172+
2173+
it('judges every member spelling as one read: optional, indexed and `has()`', () => {
2174+
for (const body of ['has(os.org)', 'os.?org.orValue({}) == {}', "os['org'].id != ''", "has(ctx.locale)"]) {
2175+
const issues = validateStackExpressions(detail(body));
2176+
expect(issues, body).toHaveLength(1);
2177+
expect(issues[0]!.where, body).toBe(WHERE);
2178+
}
2179+
});
2180+
2181+
it('one finding per option: an unbound root before a member, then members in `SCOPE_ROOTS` order', () => {
2182+
const withRoot = validateStackExpressions(detail("ctx.locale == 'en' && input.k == 1"));
2183+
expect(withRoot).toHaveLength(1);
2184+
expect(withRoot[0]!.message).toContain('reads `input`');
2185+
const twoMembers = validateStackExpressions(detail("ctx.locale == 'en' && os.env == 'prod'"));
2186+
expect(twoMembers).toHaveLength(1);
2187+
expect(twoMembers[0]!.message).toContain('reads `os.env`');
2188+
});
2189+
2190+
/**
2191+
* The allowlist is derived from the code, both ways. The real
2192+
* `buildScope` and evaluator, given the option check's context: every
2193+
* member they mount under `ctx` / `os` is accepted and evaluates, and
2194+
* every member they mount there only when ALSO given an organization and
2195+
* an environment is refused and faults. A member `buildScope` starts
2196+
* mounting from the option check's context, or one the verdict accepts
2197+
* that it no longer mounts, turns this red.
2198+
*/
2199+
it('the accepted members are exactly the ones `buildScope` mounts for the option check', () => {
2200+
const optionScope = buildScope(OPTION_CHECK_CONTEXT);
2201+
const fullScope = buildScope({ ...OPTION_CHECK_CONTEXT, org: { id: 'org_1' }, env: 'prod' });
2202+
const accepted: string[] = [];
2203+
const refused: string[] = [];
2204+
for (const root of ['ctx', 'os']) {
2205+
const mounted = Object.keys(optionScope[root] as Record<string, unknown>);
2206+
for (const member of Object.keys(fullScope[root] as Record<string, unknown>)) {
2207+
const body = `${root}.${member} != null`;
2208+
const issues = validateStackExpressions(detail(body));
2209+
const evaluated = ExpressionEngine.evaluate({ dialect: 'cel', source: body }, OPTION_CHECK_CONTEXT);
2210+
if (mounted.includes(member)) {
2211+
expect(issues, body).toEqual([]);
2212+
expect(evaluated.ok, body).toBe(true);
2213+
accepted.push(body);
2214+
} else {
2215+
expect(issues, body).toHaveLength(1);
2216+
expect(evaluated.ok, body).toBe(false);
2217+
refused.push(body);
2218+
}
2219+
}
2220+
}
2221+
expect({ accepted, refused }).toEqual({
2222+
accepted: ['ctx.user != null', 'os.user != null'],
2223+
refused: ['os.org != null', 'os.env != null'],
2224+
});
2225+
});
2226+
});
2227+
20912228
it('flags a bare-field sharing-rule condition', () => {
20922229
const issues = validateStackExpressions({
20932230
objects: [{ name: 'crm_account', fields: { region: { type: 'text' } } }],

‎packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,6 +2354,121 @@ describe('runtime authoring gate on OBJECT writes — an option visibleWhen read
23542354
});
23552355
});
23562356

2357+
/**
2358+
* [#22274] The object save door refuses a field option's `visibleWhen` that
2359+
* reads a member of `ctx` or `os` the server's option check never binds, as
2360+
* `os build` does.
2361+
*
2362+
* The option check (`evaluateOptionVisibility` in ObjectQL) fills `ctx` and
2363+
* `os` with their `user` member only. Measured before this change: an option
2364+
* whose `visibleWhen` read `os.org.id`, `os.env` or `ctx.locale` saved through
2365+
* this door, and every write that picked the option faulted (`No such key`)
2366+
* and was admitted unchecked.
2367+
*
2368+
* The refusal is in `@objectstack/lint` (the option pass's member verdict); no
2369+
* code here moves. Pinned through the REAL `saveMetaItem`:
2370+
*
2371+
* (a) each measured body is refused on an active save, a 422
2372+
* `INVALID_METADATA` carrying the build's located finding, and nothing
2373+
* lands;
2374+
* (b) control: the acting user under every ADR-0068 spelling, `record`,
2375+
* `previous` and `current_user.can(…)` save, and the row lands;
2376+
* (c) the door's issue and the build's finding are the same finding.
2377+
*
2378+
* ⚠️ As in the blocks above: this package reaches `@objectstack/lint` through
2379+
* its built `dist/`, so an edit to the rule is invisible here until
2380+
* `pnpm --filter @objectstack/lint build` has run.
2381+
*/
2382+
describe('runtime authoring gate on OBJECT writes — an option visibleWhen reading an unbound ctx/os member (#22274)', () => {
2383+
const item = (visibleWhen: unknown) => ({
2384+
name: 'fx_item',
2385+
label: 'Item',
2386+
sharingModel: 'private',
2387+
fields: {
2388+
x: { type: 'text', label: 'X' },
2389+
tier: {
2390+
type: 'select',
2391+
label: 'Tier',
2392+
options: [{ label: 'Standard', value: 'standard' }, { label: 'Gold', value: 'gold', visibleWhen }],
2393+
},
2394+
},
2395+
});
2396+
/** The card's measured bodies, each with the member path the finding names. */
2397+
const REFUSED: Array<[string, string]> = [
2398+
["os.org.id != ''", '`os.org`'],
2399+
["os.env == 'prod'", '`os.env`'],
2400+
["ctx.locale == 'en'", '`ctx.locale`'],
2401+
];
2402+
/** Where the build locates it — the option the author edits. */
2403+
const WHERE = "object 'fx_item' · field 'tier' option 'gold' visibleWhen";
2404+
2405+
const host = () => {
2406+
const { engine, rows } = makeStubEngine();
2407+
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test') as any;
2408+
return { protocol, rows };
2409+
};
2410+
const itemRows = (rows: Map<string, Row>) =>
2411+
Array.from(rows.values()).filter((r) => r.type === 'object' && r.name === 'fx_item');
2412+
const save = (protocol: any, visibleWhen: unknown) =>
2413+
protocol.saveMetaItem({ type: 'object', name: 'fx_item', item: item(visibleWhen) });
2414+
const buildFindings = (visibleWhen: unknown) => {
2415+
const stack = { objects: [item(visibleWhen)] };
2416+
return runAuthoringRules('build', { normalized: stack, parsed: stack })
2417+
.filter((f) => f.rule === EXPRESSION_INVALID);
2418+
};
2419+
2420+
it.each(REFUSED)('(a) REFUSES %s on an active save with a 422 carrying the located finding, and nothing lands', async (body, path) => {
2421+
const { protocol, rows } = host();
2422+
2423+
const err = await save(protocol, body).catch((e: any) => e);
2424+
2425+
expect(err, 'the save resolved — the door still accepts the option predicate').toBeInstanceOf(Error);
2426+
expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 });
2427+
const issues = err.issues.filter((i: any) => i.rule === EXPRESSION_INVALID);
2428+
expect(issues, `issues: ${JSON.stringify(err.issues)}`).toHaveLength(1);
2429+
expect(issues[0].path).toBe(WHERE);
2430+
expect(issues[0].severity).toBe('error');
2431+
// The named subject: the option, the field, the member, and what IS bound under its root.
2432+
expect(issues[0].message).toContain(`option 'gold' on field 'tier' reads ${path}`);
2433+
expect(issues[0].message).toContain('the `user` member and nothing else');
2434+
expect(itemRows(rows)).toEqual([]);
2435+
});
2436+
2437+
it('(b) control: the acting user under every ADR-0068 spelling, `record`, `previous` and `can` save, and the row lands', async () => {
2438+
for (const body of [
2439+
"current_user.id != ''",
2440+
"os.user.id != ''",
2441+
"'org_admin' in ctx.user.positions",
2442+
"user.id != ''",
2443+
"record.x == 'a'",
2444+
"previous.x == 'a'",
2445+
"current_user.can('fx_item', 'edit')",
2446+
"current_user.organizationId != ''",
2447+
]) {
2448+
const { protocol, rows } = host();
2449+
const result = await save(protocol, body);
2450+
expect(result.success, body).toBe(true);
2451+
expect(itemRows(rows).map((r) => r.state), body).toEqual(['active']);
2452+
}
2453+
});
2454+
2455+
it.each(REFUSED)('(c) the door and `os build` give the SAME finding for %s', async (body) => {
2456+
const { protocol } = host();
2457+
const err = await save(protocol, body).catch((e: any) => e);
2458+
const atDoor = (err.issues ?? []).filter((i: any) => i.rule === EXPRESSION_INVALID);
2459+
2460+
const atBuild = buildFindings(body);
2461+
2462+
// Non-vacuous on both sides.
2463+
expect(atBuild).toHaveLength(1);
2464+
expect(atBuild[0]!.severity).toBe('error');
2465+
expect(atDoor).toHaveLength(1);
2466+
for (const key of ['rule', 'where', 'path', 'message', 'hint'] as const) {
2467+
expect(atDoor[0][key], `door and build disagree on '${key}'`).toBe(atBuild[0]![key]);
2468+
}
2469+
});
2470+
});
2471+
23572472
/**
23582473
* [#22118] The object save door judges a stored sibling's finding against the
23592474
* STORED universe — the registry's objects WITH the written object's stored

0 commit comments

Comments
 (0)