Skip to content

Commit 7d2792a

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-16185-migration-flag-column-move-attest
2 parents 8f13389 + e4fd55d commit 7d2792a

13 files changed

Lines changed: 1414 additions & 39 deletions
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
`approval-approvers-may-resolve-empty` now covers the `manager` rung, not just the group-routed ones.
6+
7+
The rule exists for the empty-slate dead-end (#3424): an approver slate that resolves to nobody, with `lockRecord` turning that into a stranded record. It reasoned about `position` / `team` / `department` and said nothing about `{ type: 'manager' }` — which has the same failure shape and a strictly worse cause. A `position` rung resolves empty because the position is unstaffed, and an operator can staff it. A `manager` rung resolves empty because `sys_user.manager_id` is unset, and an operator **cannot** set it: the managed-update whitelist for `sys_user` is exactly `{name, image, locale}` (ADR-0092), the auth admin endpoints do not accept the column, and the Console renders no field for it. So the rule warned about the rung an author can rescue and stayed silent on the one they cannot — and `manager` is the canonical first rung of a tiered approval ladder, so the silent case was also the common one.
8+
9+
- **What fires.** A node whose approver slate is made up ENTIRELY of `{ type: 'manager' }` rungs now draws one `approval-approvers-may-resolve-empty` finding, at the same `info` tier as its `position` sibling. `manager` resolves through `sys_user.manager_id` of the record's owner and yields nobody when that column is unset; when nothing else is on the node, the request waits forever, and under the default `lockRecord` the record stays locked.
10+
- **What it does not claim.** The message states in as many words that this is a static check which cannot read the column, and that it does not assert the slate IS empty — it reports that nothing else on the node can approve if it is. A lint rule must not claim a runtime fact it did not read.
11+
- **The remedy it prescribes, with the routes graded rather than listed.** An exact diagnosis whose prescription cannot be carried out is worse than no prescription, so the hint separates what this platform provides from what it does not. A **seed, or any other system-context write**, populates the column here — both write guards gate on `isUserContextWrite` (`userId && !isSystem`), so a system-context write bypasses the managed-update whitelist by construction. **SCIM provisioning and directory sync** are named too, because a deployment running a real one may well populate the column through it — but named as a path the deployment itself supplies: this repo declares the SCIM Enterprise `manager` attribute without projecting it onto the column, and the admin bulk import does not write it either (`SYS_USER_IMPORT_UPDATE_FIELDS` is `{name, image, locale}` plus `phone_number` and `role`, and `manager_id` is listed there among the admin-surface-only columns). Editing the user in the Console is explicitly ruled out, since it cannot write the column at all. And the escape that depends on none of this stays on offer: add a fallback approver that cannot resolve empty, such as `{ type: 'org_membership_level', value: 'owner' }`.
12+
- **When it stays quiet — and on which surface.** A stack whose own seed data wires `sys_user.manager_id` on any seeded row has shown the linter that it populates the column, and the advisory is suppressed. Seed rows are the only manager-chain evidence a stack can carry, so that is the whole of what this check reads on the question. ⚠️ That suppression is **CLI-side only**. The runtime publish gate hands rules a `RuntimeStackContext` whose collections are fixed — `objects`, `permissions`, `books`, `datasets`, `pages`, and no `data` — so a Studio publish of a manager-only flow carries no seeds to read and draws the advisory however the tenant's users are wired. That is a surface asymmetry, not a broken suppressor: an `info` finding never blocks a publish, it rides the 2xx `advisories`. Noted here so a reader who seeds correctly and still sees it fire on publish does not go looking for a bug in the rule.
13+
14+
Existing verdicts are unchanged. The new arm is scoped to slates that are entirely `manager` rungs, which keeps it disjoint from the group-routed arm by construction — no node can draw both findings — and leaves every `position` verdict exactly as it was, mixed slates included: a `[position, manager]` node stays silent, as it is pinned to.
15+
16+
This is a purely additive widening of a published package's public surface — the rule begins covering a case it was silent on — so it is graded `minor`, the floor that act carries regardless of the commit type.
17+
18+
No severity moved. The finding is `info`, so it lands in the advisory channel on every consumer: `os lint` renders it as a suggestion and its exit code is unchanged (a suggestion does not fail a run even under `--strict`), and the runtime publish gate returns it on the 2xx `advisories` array rather than refusing the write. What changes is the report, not any verdict.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
Two new gating rules — `rls-predicate-unknown-field` and `rls-predicate-unknown-user-variable`: an RLS predicate that lowers correctly but names a column the object does not declare, or a `current_user.*` value nothing pre-resolves, is now an authoring-time `error`.
6+
7+
The three shipped `rls-predicate-*` rules judge a predicate's **shape** — does it parse, does it lower, does it fit the platform's CEL bounds. Nothing judged what it **points at**. Measured as four injections at one site, in one run: `billing_address.country == "US"` reported `rls-predicate-unenforceable` and `is_private == = false` reported `rls-predicate-unparseable`, while `is_private_nope == false || owner_id == current_user.id` and `is_private == false || owner_id == current_user.nope` reported **nothing at all** — from the same site the linter had just reported twice.
8+
9+
Both silent shapes are expensive rather than cosmetic, and they do **not** fail in the same direction — which is the part the card's own measurement did not reach.
10+
11+
An unresolved `current_user.*` is refused by the pushdown compiler in **every** position, including under `!` and in a trailing `||` arm, so that half always fails **closed**: `RLSCompiler` drops the policy, the layer falls back to the `RLS_DENY_FILTER` sentinel, and the object disappears for every holder of the permission set — not because they were denied but because the narrowing they were granted resolves to nothing.
12+
13+
An unknown **field** takes its direction from **position**, and one of the two is fail-**open**. `SecurityPlugin`'s field-existence safety net recognises only a *leading* `field ==` / `=` / `in` (`extractTargetField` is that shape match), so a miss there drops the policy and arms the deny sentinel — zero rows. A miss the net does not recognise — a negation (`nope != "x"`, `!(nope == 1)`, `!(nope in ['a'])`) or any arm after the first — leaves the policy **kept**, and the phantom column lowers to a negated constraint that a row without that column *satisfies* (`noValueSatisfiesNegation`: `$ne` / `$nin` / `$notContains`). The authored narrowing is then **defeated rather than enforced**: measured at 3 of 3 rows, against 1 of 3 for the real narrowing and 0 of 3 for the same phantom column in a positive position, on the read path and on the write path's `matchesFilterCondition` alike.
14+
15+
⛔ That is **not** a cross-tenant leak — tenancy is a separate layer and it holds; what is defeated is the narrowing authored inside the wall. Measured on driver-memory; driver-mongodb follows the same shared ruling; **driver-sql is NOT MEASURED** and is expected to fail closed by raising `no such column`. The runtime repair is tracked separately as #17042 and is deliberately not attempted here — these rules report the miss, in both directions, and the diagnostic says which direction applies so an author is not told "this denies everything" about a predicate that in fact matches everything.
16+
17+
- **Two rules beside the three, not a widening of them.** The existing ids say *unenforceable* / *unparseable* / *over-budget* and are correct inside that scope; they are untouched, and the two controls above still report under them and under neither new id. The prescriptions differ (rewrite the predicate / fix the column name / pre-resolve the variable), and an author who suppresses one must not thereby suppress the other. The guards are disjoint by construction: the reference pass runs only where `isSupportedRlsExpression` has already said yes.
18+
- **Where the existence answer comes from.** Field paths are read off the pushdown compiler's **own output** — the lowered `FilterCondition`'s keys are the columns the driver will be handed — and resolved through `object-graph.ts`, the shared index every field-existence rule in this package already uses. No new input path, no second parse of the predicate. The rule therefore inherits that module's three skips, each the difference between a finding and a false one: an object this stack does not define, an object with no readable field map (an ADR-0015 `external` object, an introspected datasource), and registry-injected system columns such as `created_at`, which are real at runtime and appear in no authored `fields`.
19+
- **The `current_user` set is derived, not transcribed.** It is `RESERVED_RLS_MEMBERSHIP_KEYS` from `@objectstack/spec/contracts` — the keys an `IRlsMembershipResolver` may never supply *because the kernel already owns them*. A key added there stops being reported the same day, with no edit in this package.
20+
- **§7.3.1 membership keys are left alone, and that boundary is the reason this rule can exist.** An app stages arbitrary sets into `ExecutionContext.rlsMembership` and references them as `field in current_user.<key>`; the spec documents the pattern and `rls-predicate-unparseable`'s own hint recommends it. In an `in` position an unknown key is indistinguishable from a correct one and is never reported. It is decidable in the other positions only because the merge is array-only — the sole value an app-staged key can ever hold is an array, which a scalar position cannot use on any request — so `owner_id == current_user.nope` is refused while `assigned_to_id in current_user.team_member_ids` stays silent. A key used in both positions takes the membership answer.
21+
22+
**What moves for consumers.** A stack whose RLS predicate names a renamed column or an un-pre-resolved context value built clean before and now fails `os validate` / `os lint` / `os compile`. That is the point — the policy had already stopped doing what it was written to do, denying the whole object in one position and granting every row in the other.
23+
24+
A stack whose predicates all resolve is byte-identically clean. The reading is the shipped showcase: 3 RLS clauses, all 3 judgeable against declared objects, **zero** findings — with three firing controls at the real site (an injected dangling column, an injected unknown variable, and an injected fail-open negation shape each produce exactly one finding) and two nonsense controls (an injected membership test against an unknown key, and a real-field/real-variable predicate, stay silent). `plugin-security`'s seed sets and hotcrm's built-permissions fixture also emit zero, but ⛔ **those two are not readings**: every policy target in the seeds is an object that package does not declare, and the hotcrm fixture carries no `objects` key at all, so all 71 and all 4 clauses respectively are skipped by construction. Declaring one of their objects makes the fixture report 2 — which is what a control is for.

content/docs/automation/flows.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,6 +1734,8 @@ curl -b cookies.txt -X POST \
17341734
| `GET /api/v1/automation/:name/runs/:runId` | One run's detail (404 `Execution not found`). Requires read on `sys_automation_run` — see [Observing runs](#observing-runs) |
17351735
| `POST /api/v1/automation/:name/runs/:runId/resume` | Resume a paused run — body `{ inputs, output, branchLabel }` |
17361736
| `GET /api/v1/automation/:name/runs/:runId/screen` | The pending screen of a screen-flow run |
1737+
| `POST /api/v1/automation/:name/runs/:runId/cancel` | Cancel a suspended run (ADR-0044) — `PLATFORM_ADMIN` posture only (`isSystem` bypass); body `{ reason? }`. `501 NOT_IMPLEMENTED` when the service does not declare the verb |
1738+
| `POST /api/v1/automation/:name/runs/:runId/restore-suspension` | Re-arm a run stranded by a failed resume — same gate as `cancel`; body `{ reason? }`. `501 NOT_IMPLEMENTED` when the service does not declare the verb |
17371739

17381740
**Failures answer real HTTP status codes.** A flow that does not run is not a
17391741
`200`. The route classifies five outcomes, and it reads the engine's own verdict

content/docs/permissions/system-context.mdx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,23 @@ node scripts/isystem-census.mjs # the summary below
310310
node scripts/isystem-census.mjs --json # every site, its kind and its package
311311
```
312312

313+
**Every number below marked ✅, plus the four sentences elsewhere on this page
314+
that restate the same figures (the opening headline, the plugin-sharing share,
315+
the #4707 ruling quote), is *generated*, never hand-maintained.** `pnpm
316+
gen:system-context-census` (`node scripts/check-system-context-census.mjs
317+
--fix`) re-derives the census and rewrites every declared count that has
318+
drifted from it — the exact computation the check below already runs, applied
319+
as a write instead of a comparison. ⛔ **Never hand-retype one of these
320+
digits.** Two branches each independently — and correctly, for their own
321+
tree — bumping the same sentence to the same number is exactly how this page
322+
once went silently wrong: the edits are textually identical, git merges them
323+
clean with no conflict, and the merged total is neither side's number
324+
(#16919). Run the generator instead, against the tree you actually want
325+
counted, and it fails loudly rather than writing a partial page if a sentence
326+
no longer parses. `--fix` still cannot add or drop a **row** — a site that
327+
arrived or vanished is a human's editorial call, same as always — it only
328+
keeps the aggregate counts in lockstep with whatever rows exist.
329+
313330
**⛔ Not a `grep`.** A text scan is where a census *starts* and it cannot be where
314331
one ends: it returns prose inside comments and strings, the three unrelated
315332
metadata fields, and the `isSystemObject` / `isSystemObjectName` /

packages/lint/src/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,11 +346,17 @@ export type {
346346
// as an authorization and behaves as a blanket refusal. The verdict is
347347
// `isSupportedRlsExpression` — the runtime's own, hoisted into
348348
// `@objectstack/formula` in the same change so lint can reach it.
349+
// [#16119] …and the REFERENCE half of the same gate: the three ids above judge a
350+
// predicate's SHAPE and never what it POINTS AT, so a policy naming a renamed
351+
// column or an un-pre-resolved `current_user.*` value was reported by nothing
352+
// while failing closed on the whole object for every holder of the set.
349353
export {
350354
validateRlsPredicateEnforceability,
351355
RLS_PREDICATE_UNENFORCEABLE,
352356
RLS_PREDICATE_UNPARSEABLE,
353357
RLS_PREDICATE_OVER_BUDGET,
358+
RLS_PREDICATE_UNKNOWN_FIELD,
359+
RLS_PREDICATE_UNKNOWN_USER_VARIABLE,
354360
} from './validate-rls-predicate-enforceability.js';
355361
export type {
356362
RlsPredicateFinding,

0 commit comments

Comments
 (0)