Uh oh!
There was an error while loading. Please reload this page.
fix(formula): fail closed on a null MEMBER of a resolved membership array in the CEL pushdown (#13496) - #13630
Conversation
…rray in the CEL pushdown lowerMembership checked only Array.isArray and emitted the list verbatim, so a null MEMBER of a resolved membership variable reached a security $in while a null SCALAR variable took the pinned unresolved-variable fail-closed path. It now refuses the member with the same reason, in every polarity. Refusing rather than stripping: `not in` lowers to $not wrapping $in, and $in: [] matches nothing, so stripping inverts into allow-all under negation. Refusing throws before any $not wrapper is built, so no polarity threading is needed in the lowerer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
…ST_DEBT errors The module-level ok() pins its second argument to the exact shape of VARS, so the partial contexts this suite builds were 4 new TS2345 on a shrink-only ledger. Same assertion, same throw, widened only where this suite needs it; re-measured @objectstack/formula TEST_DEBT back to the frozen 17. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8560f53d5587879152caaa20ae4cfaa1d273857c && git checkout 8560f53d5587879152caaa20ae4cfaa1d273857c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin eb64351f092071aeccfb4eb7fb8492e8d08c4362 1cdf3c14efdfbccb7c290629d4acf984c969302a && git checkout -B drift-repro eb64351f092071aeccfb4eb7fb8492e8d08c4362 && git merge --no-ff 1cdf3c14efdfbccb7c290629d4acf984c969302a
node scripts/docs-audit/affected-docs.mjs --json eb64351f092071aeccfb4eb7fb8492e8d08c4362 |
zhuangjianguo
commented
Aug 31, 2026
条款② contract review — verdict: PASS (reviewed at head 1. The refusal denies at every consumer — verified by enumeration. Repo-wide at head, 2. #13357 boundary holds, verified beyond the pin. The guard's precondition is 3. Unwinding claim verified in control flow and under deeper composition. One 4. Narrowing is not over-broad, and the differential is exact. A 16-case attack suite at head: all pass — 5. Diagnosability (observation only — the ruling settled the trade). The refusal's Formula suite reproduced green at head (655/655, 25 files). Review worktree restored byte-identical ( Generated by Claude Code |
zhuangjianguo
commented
Aug 31, 2026
PM review — ACCEPT. Clause ② returned PASS. Release on the last check.
Clause ② — PASS, and the attack behind it was realThe review's strongest result is the one I asked for hardest: no consumer swallows the refusal. Repo-wide enumeration found exactly three non-test callers of
|
Uh oh!
There was an error while loading. Please reload this page.
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 33357337261 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
zhuangjianguo
commented
Aug 31, 2026
Dequeued by a failure that is not this PR's — standing down, onceThis PR was dequeued from the merge queue at Failing check:
Why it is not this PR'sThis PR changes three files — The cause is #13645: ⛔ Not calling this a flake, and not spending the re-runWhen #13645 was filed, "reproduces identically" was recorded as UNMEASURED — the sanctioned re-run had returned 403 while sibling jobs were still in flight. It is measured now, by a stronger route than a re-run: the identical failure, at the identical line, occurred on two independent PRs with disjoint diffs — this one (04:33:56Z, That also rules out a re-run as the remedy. The failure mode is monotonic, not random — it gets worse with every file added to the repo and there is no random component to re-roll — so a re-run would re-fail and cost a cycle. The one sanctioned re-run stays unspent on this PR. What happens nextNo fix for #13645 existed at the time of this comment, so there is nothing to port into this PR yet. One is now dispatched, against #13645, as its own change in This PR is otherwise Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13496
Implements the maintainer's ruling of 2026-08-31 (总监席第 5 场决裁批 #1, verbatim 「同意」, option A). The ruling is quoted unchanged, per the repo's rule that a Chinese ruling keeps its original wording:
The asymmetry this closes
compileCelToFilteralready fails closed when acurrent_user.*variable resolves toundefined/null— the module docblock calls it "the no active org fail-closed path" and it is pinned for the SCALAR case.lowerMembershipdid not apply the same discipline one level in: it checked onlyArray.isArray(value)and emitted the list verbatim, so a null MEMBER of a resolved membership array went straight into a security$in. The one shape that IS a permission predicate was the one shape that did not fail closed.lowerMembershipnow refuses anull/undefinedmember of a variable-resolved membership array with the sameunresolved-variablereason, whichplugin-security/rls-compiler.tsalready turns intoRLS_DENY_FILTER(if (!result.ok) return null;, line 343).Execution point 2 — both polarities, and why it is the crux
A positive-polarity pin alone is green for both candidate repairs and therefore pins nothing about the one that was ruled on. The rejected alternative — stripping the unresolved member — is safe in POSITIVE polarity (an
$inover the surviving members never grants more than those members grant) and inverts under the supportednot inform:!(x in y)lowers to$notwrapping$in, and$in: []matches nothing on every backend, so$not { $in: [] }matches the WHOLE table. Stripping is fail-OPEN exactly where the predicate is a blocklist.Twelve pins were added. Eight assert the refusal, in both polarities:
id in current_user.org_user_ids['u_me', null]unresolved-variableid in current_user.org_user_ids[null]unresolved-variableid in current_user.org_user_ids['u_me', undefined]unresolved-variable!(id in current_user.org_user_ids)['u_me', null]unresolved-variable!(id in current_user.org_user_ids)[null]unresolved-variable, not$not{$in:[]}!(...) || owner == current_user.id[null]unresolved-variable!(...) && owner == current_user.id['u_me', null]unresolved-variableFour more are non-regression controls that must NOT move: a fully resolved list still compiles in both polarities; an empty list still compiles to
$in: []in both polarities (a legitimate declared predicate); an AUTHORED literal null in a list is untouched;isPushdownableCelis untouched.The PM's Zone 2 assumption — tested, and it HOLDS
The expectation was that the guard would be a few lines inside
lowerMembershipwith no polarity threading, because failing closed refuses before polarity exists. Measured true, and the mechanism is explicit:lowerCelAstwraps the whole descent in onetry/catch, andlowerCondition's!_case builds its$notonly from the value its recursive call returns. A throw fromlowerMembershiptherefore unwinds before any wrapper exists, so!,&&and||all collapse to the singleunresolved-variableresult. The guard is 6 lines and reads no polarity. The measurement is the eight-row table above plus the "identical reason" pin.Ablation
The implementation was committed first, then the guard body was deleted and a marker injected in its place. The mutation was proven on disk in both directions (injected marker count 1, deleted text count 0, on-disk hash
a32c33dadiffering from the HEAD blobb87311f1) before anything was measured.Ablated result: 8 failed, 56 passed. Exactly the eight refusal pins went red; all four non-regression controls stayed green. The sharpest reading is the identity pin, which reported
['unresolved-variable', 'ok', 'ok']— the scalar path refusing while both membership polarities compiled, i.e. the card's asymmetry reproduced.Restore ran under
trap ... EXIT INT TERMwith an absolute repo root andgit checkout HEAD -- path(never the bare form, which restores from the polluted index). Proven by whole-treegit status --porcelainempty,git diff HEADempty, and a HEAD-blob hash match:b87311f1d259d517c3342dd474d2d6830e21457aon both sides, non-empty.No rebuild leg was needed and none is claimed: the suite imports
./cel-to-filter, a same-package relative specifier, so vitest compiles the mutated source directly rather than resolving a dependency'sexportstodist/. The red result is itself the proof — a stale-distablation stays green.Verification — all at final commit
1cdf3c14efpnpm --filter '@objectstack/formula^...' build—check-dts-emitted: @objectstack/spec - 34/34 declared declaration file(s) present. Dependency closure built before any reading.pnpm --filter @objectstack/formula test—Test Files 25 passed (25),Tests 655 passed (655).pnpm --filter @objectstack/formula typecheck— exit 0, script name echoed.plugin-security1694 passed / 92 files,plugin-sharing678 / 30,lint2360 / 85,service-analytics1805 / 83 — 6537 tests, 290 files, all green, each closure built first.eslint . --no-inline-config --format jsonover 5554 files — 0 errors, 0 warnings.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(29 path-derived + the convention-triggered set for "adds or edits a test file"): all green except two that could not measure locally, recorded as NOT MEASURED rather than as passes —check-test-completeness(exit 3,PREREQUISITE NOT MET, it grades a savedturbo run testlog CI tees) andcheck:dual-build-cjs-loads(exit 3, its own line: "This is NOT a pass: nothing was measured", 52 packages have nodist; it needs a fullpnpm build). Both are CI's on this PR.check:type-check-debt— refused globally, measured narrowlyThe gate refuses outright with 55 unbuilt workspace dependencies of the ledgered packages, which is a refusal to measure and not a red. Only
@objectstack/formula's ledger entry can move from this diff, and formula's own closure (just@objectstack/spec) IS built — so its generated re-measure project was reproduced faithfully from the gate's ownremeasureProjectand run directly.That replica is calibrated, not asserted: the first run scored 21 and itemised
TS2591 x6, TS2345 x7, TS2352 x3, TS1470 x2, TS2339 x2, TS2739 x1, against a frozen ledger value of 17 — the pre-existing 17 exactly, plus 4 newTS2345this branch had introduced. The module-levelok()helper pins its second argument to the exact shape ofVARS, which the partial contexts in the new suite cannot satisfy. Repaired at the source (a locally widened helper, same assertion and same throw) rather than by touching the shrink-only baseline. Re-measured: 17, matching the ledger's frozen value.Scope boundaries held
$in:[null]/$nin:[null]disagree while$null/$ne:nullagree #13357 (what a NULL comparand should select). The ruling states the two are independent and neither waits. Nothing here decides what$in: [null]selects — the guard refuses before a filter exists, and an authored literal null in a list is deliberately untouched, with a pin recording only that the compiler still lowers it.isEmptyMembershipFilterdeny guard is polarity-blind: an emptied membership under a supportednot incompiles to allow-all instead of the deny sentinel #13552 (isEmptyMembershipFilteris polarity-blind). Read for context, not repaired here; it is reachable with no null anywhere and survives this change independently.Clause 2 reading
Concur that clause 2 FIRES: this changes what a security pushdown refuses, so it warrants contract review at
CONTRACT_REVIEW_TIERbefore ready. The ruling's point 3 (zero visible production change today) is material FOR the reviewer, not a reason to skip — and the reviewer's attention is best spent on one question: whether refusing at the compiler is the right altitude given the lockout it implies. A user whose membership array carries one unresolvable member now loses the whole read scope, including rows their resolved members legitimately grant, and the loss is silent at the RLS layer (the policy drops to the deny sentinel). That cost is what the ruling accepted, and it is measured to be unreachable from first-party providers today:resolve-authz-context.tsfilters non-strings out oforg_user_idsand the kernel spec declaresorg_user_ids: z.array(z.string()). The residual surface is host-supplied —compileCelToFilter'svariablesis a documented public option, and two call sites readorg_user_idsthrough casts that bypass the declared type.I read the scope of that review as
packages/formula/src/cel-to-filter.tsonly. The R9 report argued for widening it to coverrls-compiler.ts'sisEmptyMembershipFilter; that was sound while stripping was still a live candidate, since stripping depended on that guard. With fail-closed ruled, the repair no longer leans on it at all, so it is context rather than half of this change's accept/reject surface.Generated by Claude Code