You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
service-analytics: the ROW-SCOPE bridge collapses the same three-way it was just fixed on the object-level one — a throwing getService('security') runs the query with NO row scope instead of refusing #16918
Filed by the domain:services PM seat (#6021), promoting a finding the #16645 patch round recorded as "noted, not filed". The dev's reasons for not fixing it in that PR are sound and I agree with them; its reason for not filing is the part I am overriding, because the defect outlives the PR that found it.
The defect
packages/services/service-analytics/src/plugin.ts bridges to plugin-security in two places. The contract review of PR #16860 (F3) examined the object-level bridge and required its silent-admit corners to deny. Those are now closed: a throwing getService('security') and a registered service exposing neither canReadObject nor explain both deny with an error-level report, while an absent service still admits — the state where the analytics door and the /data door agree, kept as an explicit negative control.
The row-scope bridge (getReadScope) in the same file has the identical three-way shape and still collapses it: a throwing getService('security') leaves trySecurity() returning undefined, and the query then runs with no row scope at all rather than refusing.
So after #16860 lands, one door of the same file fails closed on a throwing resolver and its neighbour fails open — and the one that fails open is the one carrying row-level policy.
Stated so whoever picks this up does not re-litigate it:
It is pre-existing, not introduced by that PR.
The reviewer examined that bridge and did not name it, so acting would have been widening a patch round past its review.
Most substantively: closing it changes whether affected queries run unscoped or refuse, and fix(analytics): ask the object-level read grant before serving an inline dataset — one admission verdict on every driver #16860 deliberately keeps every persona's tenant behaviour byte-identical to origin/main because triage condition 5 (which tenant wall is the platform's — plugin-security's posture-gated Layer 0, or driver-sql's posture-independent auto-scope) is an escalated maintainer decision. Moving row-scope behaviour inside that PR would have silently pre-empted it.
Why it is nonetheless a card
A throwing service resolver is not exotic — it is what a boot-order fault, a mis-registered plugin or a failing dependency looks like at exactly the moment a security bridge is consulted. In that state the analytics path currently answers rows with no row-level policy applied, and nothing says so: trySecurity() returning undefined is indistinguishable from "no security plugin is installed", which is a legitimate configuration.
That is the same conflation the object-level fix just removed, one call site over. Leaving it recorded only in a merged PR's acceptance notes means the next reader of plugin.ts sees two adjacent bridges with different failure postures and no explanation.
Suggested shape (not a ruling)
Make getReadScope's resolution three-way the way the object-level bridge now is — absent service admits (configuration), throwing resolver and method-less service deny or refuse — and report at error level. Whether "deny" here means an empty result, a refusal envelope, or RLS_DENY_FILTER is the part that needs deciding, and it may want to wait on, or be decided with, triage condition 5 (#16645) so the two walls do not get a third answer.
⚠️ Whoever takes it: confirm first whether the object-level bridge's landed shape is still what #16860 merged with — that PR was under contract review when this was filed.
Filed by the
domain:servicesPM seat (#6021), promoting a finding the #16645 patch round recorded as "noted, not filed". The dev's reasons for not fixing it in that PR are sound and I agree with them; its reason for not filing is the part I am overriding, because the defect outlives the PR that found it.The defect
packages/services/service-analytics/src/plugin.tsbridges toplugin-securityin two places. The contract review of PR #16860 (F3) examined the object-level bridge and required its silent-admit corners to deny. Those are now closed: a throwinggetService('security')and a registered service exposing neithercanReadObjectnorexplainboth deny with an error-level report, while an absent service still admits — the state where the analytics door and the/datadoor agree, kept as an explicit negative control.The row-scope bridge (
getReadScope) in the same file has the identical three-way shape and still collapses it: a throwinggetService('security')leavestrySecurity()returningundefined, and the query then runs with no row scope at all rather than refusing.So after #16860 lands, one door of the same file fails closed on a throwing resolver and its neighbour fails open — and the one that fails open is the one carrying row-level policy.
Why it was correctly left out of #16860
Stated so whoever picks this up does not re-litigate it:
origin/mainbecause triage condition 5 (which tenant wall is the platform's — plugin-security's posture-gated Layer 0, or driver-sql's posture-independent auto-scope) is an escalated maintainer decision. Moving row-scope behaviour inside that PR would have silently pre-empted it.Why it is nonetheless a card
A throwing service resolver is not exotic — it is what a boot-order fault, a mis-registered plugin or a failing dependency looks like at exactly the moment a security bridge is consulted. In that state the analytics path currently answers rows with no row-level policy applied, and nothing says so:
trySecurity()returningundefinedis indistinguishable from "no security plugin is installed", which is a legitimate configuration.That is the same conflation the object-level fix just removed, one call site over. Leaving it recorded only in a merged PR's acceptance notes means the next reader of
plugin.tssees two adjacent bridges with different failure postures and no explanation.Suggested shape (not a ruling)
Make
getReadScope's resolution three-way the way the object-level bridge now is — absent service admits (configuration), throwing resolver and method-less service deny or refuse — and report at error level. Whether "deny" here means an empty result, a refusal envelope, orRLS_DENY_FILTERis the part that needs deciding, and it may want to wait on, or be decided with, triage condition 5 (#16645) so the two walls do not get a third answer.Provenance
POST /analytics/dataset/querywithout the object-level grant or the tenant wall — a user with NO read grant on an object gets its row count (200) whereGET /data/OBJECTanswers 403; the memory driver's path refuses both #16645); recorded in that PR's## 验收备注rather than fixed or silently dropped.tenancyblock — its guard only refuses an explicitenabled: true, so memory-driver runs show cross-organization rows a SQL driver refuses #16589 (in-memory driver ignores the sametenantId),objectstack-ai/ats#39— triage's standing note on this family is that the tenant wall must have exactly ONE implementation every read path inherits.