Skip to content

Commit 69cb2dd

Browse files
committed
fix(lint)!: refuse an option visibleWhen member of ctx/os the option check never binds (#22274)
WIP: the member arm of the option-predicate verdict; pins follow. Claude-Session: https://claude.ai/code/session_01LAi5BVvQNiYzepSAcsoFLK Co-authored-by: Claude <noreply@anthropic.com>
1 parent bb4f5cc commit 69cb2dd

1 file changed

Lines changed: 124 additions & 6 deletions

File tree

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

Lines changed: 124 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -579,9 +579,11 @@ function rulePredicates(rule: AnyRec, path: string, depth = 0): RulePredicate[]
579579
* set in #6713 — it is the fourth name `buildScope` mounts the same object
580580
* under, and the allowlist would have rejected it anyway; what the user tier
581581
* decides is only WHICH prescription it gets, and the user one is right for
582-
* `os.user`. `os.org` / `os.env` land in the same tier because the option
583-
* surface named by the first prescription binds the whole `os` namespace, not
584-
* only its `user` member.)
582+
* `os.user`. `os.org` / `os.env` land in the same tier too. The option
583+
* surface named by the first prescription does NOT bind them: the server's
584+
* option check fills `os` with its `user` member only, so a predicate moved
585+
* there meets that surface's member verdict (#22274), which names what is
586+
* bound instead.)
585587
*
586588
* `ctx` is judged as a WHOLE root, not only in `ctx.user` form, because at
587589
* this surface that is simply what is true: `buildScope` creates the `ctx`
@@ -1107,9 +1109,55 @@ function fieldTraversalMessage(
11071109
*
11081110
* ⛔ A root joins this list in the same change that binds it in
11091111
* `evaluateOptionVisibility`, never before.
1112+
*
1113+
* Two of these roots are accepted WHOLE here but bound only IN PART: `ctx` and
1114+
* `os` carry just their `user` member at the option check. What they carry is
1115+
* {@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}'s to say (#22274).
11101116
*/
11111117
const OPTION_VISIBLE_WHEN_BOUND_ROOTS: readonly string[] = ['record', 'previous', 'current_user', 'user', 'ctx', 'os'];
11121118

1119+
/**
1120+
* [#22274] The MEMBERS the option check binds under the two namespace roots
1121+
* {@link OPTION_VISIBLE_WHEN_BOUND_ROOTS} accepts whole, read off the same
1122+
* call as that list.
1123+
*
1124+
* `evaluateOptionVisibility` (`rule-validator.ts`) hands the evaluator
1125+
* `{ record, previous, user, permissions }`. `@objectstack/formula`'s
1126+
* `buildScope` builds `ctx` and `os` as containers, and from that context it
1127+
* fills each with one member, `user` (ADR-0068 D1's alias of `current_user`,
1128+
* the same `EvalUser` object). `os` gains `org` only from an `org` in the
1129+
* context, and `os.env` only from an `env`. The option check passes neither,
1130+
* and `ctx` has no other source in `buildScope` at all. So `os.org.id`,
1131+
* `os.env` and `ctx.locale` are unbound at the option check, though `os.org`
1132+
* is bound at other sites (a `formula` field, an expression `defaultValue`, a
1133+
* seed value) and `os.env` at the seed loader. Measured through the built
1134+
* `evaluateValidationRules` with an authenticated caller, each faults
1135+
* (`No such key: org` / `env` / `locale`) and the value is admitted.
1136+
*
1137+
* Only `ctx` and `os` are listed. `current_user` and `user` ARE the `EvalUser`;
1138+
* their members are that object's own fields, the same at every site that
1139+
* binds a user, so no member of theirs is unbound here in particular. That is
1140+
* also why this verdict stops at the first member: `ctx.user.positions` reads
1141+
* the `EvalUser` and is judged like `current_user.positions`, not here.
1142+
*
1143+
* The test suite derives this list from the real `buildScope` and evaluator,
1144+
* given the option check's context, so a member `buildScope` starts mounting
1145+
* there, or one this list accepts that it never mounts, turns that test red.
1146+
* ⛔ A member joins this list in the same change that binds it in
1147+
* `evaluateOptionVisibility`, never before.
1148+
*/
1149+
const OPTION_VISIBLE_WHEN_BOUND_MEMBERS: Readonly<Record<string, readonly string[]>> = {
1150+
ctx: ['user'],
1151+
os: ['user'],
1152+
};
1153+
1154+
/** A CEL member path, spelled for a message: `` `root.member` ``. */
1155+
function celMemberPath(root: string, member: string): string {
1156+
// Assembled with `+`: #5017's receiver scan reads string literals too, so a
1157+
// literal CEL path in a message would register its root as a read receiver.
1158+
return '`' + root + '.' + member + '`';
1159+
}
1160+
11131161
/**
11141162
* [#22157] The root verdict for a select option's own `visibleWhen`: a root
11151163
* the server's option check does not bind ({@link OPTION_VISIBLE_WHEN_BOUND_ROOTS})
@@ -1137,8 +1185,19 @@ const OPTION_VISIBLE_WHEN_BOUND_ROOTS: readonly string[] = ['record', 'previous'
11371185
* records (#6713): a root added to `SCOPE_ROOTS` is judged here the day it
11381186
* lands, with no second list to copy it into.
11391187
*
1188+
* ## Members of a bound root (#22274)
1189+
*
1190+
* `ctx` and `os` pass the root test, but the option check fills them with
1191+
* their `user` member only ({@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}). A
1192+
* predicate that reads any other member of either (`os.org.id`, `os.env`,
1193+
* `ctx.locale`) is refused by the same verdict, located at the same option,
1194+
* when it reads no unbound root. The members a source reads are
1195+
* `@objectstack/formula`'s `analyzeRelationshipTraversals` reading of that
1196+
* root, so `os.org`, `os.?org`, `os['org']` and `has(os.org)` are one read. A
1197+
* computed key (`os[k]`) names no member and is not judged.
1198+
*
11401199
* `null` = nothing to report: the source does not parse (the syntax pass owns
1141-
* that), or every root it reads is one the option check binds.
1200+
* that), or every root and member it reads is one the option check binds.
11421201
*/
11431202
function optionVisibleWhenRootIssue(
11441203
objectName: string | undefined,
@@ -1151,7 +1210,7 @@ function optionVisibleWhenRootIssue(
11511210
const kept = SCOPE_ROOTS.filter(
11521211
(r) => !OPTION_VISIBLE_WHEN_BOUND_ROOTS.includes(r) && roots.roots.includes(r),
11531212
);
1154-
if (kept.length === 0) return null;
1213+
if (kept.length === 0) return optionVisibleWhenMemberIssue(objectName, field, option, source, roots.roots);
11551214
// One issue per option even when the predicate reads two unbound roots, in
11561215
// `SCOPE_ROOTS` order: stable, never AST walk order. The author fixes one
11571216
// and the next run names the other.
@@ -1175,6 +1234,64 @@ function optionVisibleWhenRootIssue(
11751234
};
11761235
}
11771236

1237+
/**
1238+
* [#22274] The member half of {@link optionVisibleWhenRootIssue}: a member of
1239+
* `ctx` or `os` the option check does not bind
1240+
* ({@link OPTION_VISIBLE_WHEN_BOUND_MEMBERS}). One finding per option, the
1241+
* first unbound member in `SCOPE_ROOTS` order and then in name order: stable,
1242+
* never AST walk order.
1243+
*
1244+
* The message names the member, says what the option check DOES bind under
1245+
* that root, and gives the remedy that is true for the member. For `os.org`
1246+
* that is `current_user.organizationId`: the engine builds the acting user
1247+
* with the caller's organization id (`null` outside one), so that one
1248+
* organization fact IS bound at the option check.
1249+
*/
1250+
function optionVisibleWhenMemberIssue(
1251+
objectName: string | undefined,
1252+
field: string,
1253+
option: string,
1254+
source: string,
1255+
roots: readonly string[],
1256+
): { root: string; message: string } | null {
1257+
for (const root of SCOPE_ROOTS) {
1258+
if (OPTION_VISIBLE_WHEN_BOUND_MEMBERS[root] === undefined || !roots.includes(root)) continue;
1259+
const found = analyzeRelationshipTraversals(source, root);
1260+
if (found === null) continue;
1261+
const { traversals, bareFields, multiHopFields } = found;
1262+
const member = [...new Set([...Array.from(traversals, ([m]) => m), ...bareFields, ...multiHopFields])]
1263+
.sort()
1264+
.find((m) => !OPTION_VISIBLE_WHEN_BOUND_MEMBERS[root]!.includes(m));
1265+
if (member === undefined) continue;
1266+
const path = celMemberPath(root, member);
1267+
const owner = objectName ? `'${objectName}'` : 'this object';
1268+
const prescription = root === 'os' && member === 'org'
1269+
? `The option check binds no organization. The one organization fact it carries is the acting ` +
1270+
`user's: ${celMemberPath('current_user', 'organizationId')} holds the caller's organization ` +
1271+
`id (\`null\` outside one), so compare that instead. Any other fact about the organization ` +
1272+
`cannot gate an option; read a column ${owner} declares.`
1273+
: root === 'os' && member === 'env'
1274+
? `The option check binds no deployment environment (${path} is bound only where an ` +
1275+
`evaluation is given one, such as a seed value), so the choices a record offers cannot ` +
1276+
`depend on it there. Gate on a column ${owner} declares, or on the acting user as ` +
1277+
`\`current_user\`.`
1278+
: `Whatever other evaluation sites bind under \`${root}\`, the option check binds only its ` +
1279+
`\`user\` member. Rewrite the predicate against \`record\` (plus \`previous\`), or against ` +
1280+
`the acting user as \`current_user\`.`;
1281+
return {
1282+
root,
1283+
message:
1284+
`\`visibleWhen\` of option ${option} on field '${field}' reads ${path}, but the server's ` +
1285+
`option check binds only \`record\`, \`previous\` and the acting user (\`current_user\`, and ` +
1286+
`its ADR-0068 aliases \`user\`, \`ctx\` and \`os\`), and under \`${root}\` it binds the ` +
1287+
`\`user\` member and nothing else, so ${path} is unbound there. ` +
1288+
`${FIELD_TRAVERSAL_CONSEQUENCE['option visibleWhen']}; a \`has()\` test or an optional read ` +
1289+
`of it finds it unset on every write. ${prescription}`,
1290+
};
1291+
}
1292+
return null;
1293+
}
1294+
11781295
/**
11791296
* [#20078] The master object of an object with exactly ONE `master_detail`
11801297
* relationship — the record the field level binds as `parent` — or `undefined`
@@ -2139,7 +2256,8 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
21392256
// every write that picked the option and was admitted unchecked. So the
21402257
// two surfaces now differ by exactly what their evaluators bind: this
21412258
// one accepts the acting user and refuses `parent`, the field-rule slots
2142-
// the other way round.
2259+
// the other way round. [#22274] The same verdict judges the members of
2260+
// `ctx` and `os`, which the option check fills with `user` only.
21432261
//
21442262
// [#22032, pass 3] NOT fenced on an object write: the option's `check`,
21452263
// its root verdict (#22157) and its traversal refusal are the pass the

0 commit comments

Comments
 (0)