From 999f2db2244e70e6a60a4610685a7444bcda6bab Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 08:34:32 +0000 Subject: [PATCH 1/2] feat(spec,security): nav landing exclusivity + field-permission predicate guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two guards completing objectui#2251 / objectui ADR-0055: spec — NavigationItemSchema rejects object items combining filters with recordId or viewName (superRefine on the union member; base schema stays extendable). Runtime precedence would silently ignore the extras — a stale recordId hijacking a configured filters slice — so the ambiguous shape is now unwritable (ADR-0053 correct-by-construction). The legacy recordId+viewName combination stays tolerated (documented). 4 new schema tests; api-surface unchanged. plugin-security — anti filter-oracle predicate guard. FieldMasker only masks RESULTS; filtering/sorting/grouping/aggregating by a hidden field still leaked its values through row presence. The middleware now rejects (403, reason: field_predicate_denied) caller queries whose where/orderBy/groupBy/having/aggregations/windowFunctions reference a non-readable field — evaluated against the caller's AST BEFORE RLS injection so RLS policies may keep referencing hidden fields. Rejection over silent dropping: removing an $and branch widens results and re-opens the oracle (Salesforce FLS errors the same way). 10 new unit tests; plugin-security suite 183/183. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018m3GX7EMKNPDZuee152EKK --- ...filters-exclusivity-and-predicate-guard.md | 10 ++ packages/plugins/plugin-security/src/index.ts | 1 + .../src/predicate-guard.test.ts | 104 +++++++++++++ .../plugin-security/src/predicate-guard.ts | 139 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 18 +++ packages/spec/src/ui/app.test.ts | 47 ++++++ packages/spec/src/ui/app.zod.ts | 34 ++++- 7 files changed, 351 insertions(+), 2 deletions(-) create mode 100644 .changeset/nav-filters-exclusivity-and-predicate-guard.md create mode 100644 packages/plugins/plugin-security/src/predicate-guard.test.ts create mode 100644 packages/plugins/plugin-security/src/predicate-guard.ts diff --git a/.changeset/nav-filters-exclusivity-and-predicate-guard.md b/.changeset/nav-filters-exclusivity-and-predicate-guard.md new file mode 100644 index 0000000000..245d5261f1 --- /dev/null +++ b/.changeset/nav-filters-exclusivity-and-predicate-guard.md @@ -0,0 +1,10 @@ +--- +'@objectstack/spec': minor +'@objectstack/plugin-security': minor +--- + +feat(spec,security): make ambiguous nav landings unrepresentable + close the field-permission filter oracle (objectui#2251, objectui ADR-0055). + +**spec — `ObjectNavItem` target exclusivity.** `NavigationItemSchema` now rejects an object nav item that combines `filters` with `recordId` or `viewName` (custom issue on `filters` with the fix in the message). Runtime precedence would silently ignore the extras — a stale `recordId` hijacking a configured `filters` slice — so the ambiguous combination is now unwritable (ADR-0053 correct-by-construction). FROM `{ filters, viewName }` / `{ filters, recordId }` TO exactly one landing field; the legacy `recordId` + `viewName` combination stays tolerated (documented: `viewName` is ignored). `filters` shipped in the same unreleased minor, so no released metadata is affected. + +**plugin-security — field-level predicate guard.** `FieldMasker` strips non-readable fields from RESULTS, but predicates still leaked their values: filtering / sorting / grouping / aggregating by a hidden field changes row presence (a filter oracle — probe `salary >= X` even though the column is masked). The security middleware now rejects (403 `PermissionDeniedError`, `reason: 'field_predicate_denied'`) any caller query whose `where` / `orderBy` / `groupBy` / `having` / `aggregations` / `windowFunctions` reference a field the caller cannot read — evaluated against the caller's AST **before** RLS injection, so RLS policies may keep referencing hidden fields (e.g. `owner_id`). Rejection over silent predicate dropping: removing an `$and` branch widens results and re-opens the oracle. New exports: `assertReadableQueryFields`, `collectQueryFields`, `collectConditionFields`. diff --git a/packages/plugins/plugin-security/src/index.ts b/packages/plugins/plugin-security/src/index.ts index 7c88732189..17aab180ee 100644 --- a/packages/plugins/plugin-security/src/index.ts +++ b/packages/plugins/plugin-security/src/index.ts @@ -11,6 +11,7 @@ export { SecurityPlugin } from './security-plugin.js'; export { PermissionEvaluator } from './permission-evaluator.js'; export { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; export { FieldMasker } from './field-masker.js'; +export { assertReadableQueryFields, collectQueryFields, collectConditionFields } from './predicate-guard.js'; export { PermissionDeniedError, isPermissionDeniedError } from './errors.js'; export { securityObjects, diff --git a/packages/plugins/plugin-security/src/predicate-guard.test.ts b/packages/plugins/plugin-security/src/predicate-guard.test.ts new file mode 100644 index 0000000000..ce2465b26f --- /dev/null +++ b/packages/plugins/plugin-security/src/predicate-guard.test.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** Field-level predicate guard — anti filter-oracle (objectui#2251). */ + +import { describe, it, expect } from 'vitest'; +import { + collectConditionFields, + collectQueryFields, + assertReadableQueryFields, +} from './predicate-guard.js'; +import { isPermissionDeniedError } from './errors.js'; + +const HIDDEN_SALARY = { salary: { readable: false, editable: false } }; + +describe('collectConditionFields', () => { + it('collects implicit equality, operators, and logical nesting', () => { + const fields = collectConditionFields({ + status: 'open', + salary: { $gte: 100000 }, + $or: [{ priority: 'high' }, { $not: { archived: true } }], + $and: [{ due_date: { $lte: '2026-12-31' } }], + }); + expect([...fields].sort()).toEqual(['archived', 'due_date', 'priority', 'salary', 'status']); + }); + + it('gates dotted paths on the first segment', () => { + expect([...collectConditionFields({ 'owner.name': 'x' })]).toEqual(['owner']); + }); +}); + +describe('collectQueryFields', () => { + it('covers where / orderBy / groupBy / having / aggregations / window functions', () => { + const fields = collectQueryFields({ + where: { status: 'open' }, + orderBy: [{ field: 'salary', order: 'desc' }], + groupBy: ['department', { field: 'hired_at', dateGranularity: 'month' }], + having: { headcount: { $gt: 3 } }, + aggregations: [{ function: 'sum', field: 'bonus', alias: 'total', filter: { region: 'emea' } }], + windowFunctions: [ + { function: 'row_number', alias: 'r', over: { partitionBy: ['team'], orderBy: [{ field: 'score', order: 'desc' }] } }, + ], + }); + expect([...fields].sort()).toEqual([ + 'bonus', 'department', 'headcount', 'hired_at', 'region', 'salary', 'score', 'status', 'team', + ]); + }); + + it('does NOT collect the projection — masked selects are harmless', () => { + const fields = collectQueryFields({ fields: ['salary', 'name'], where: { status: 'open' } }); + expect(fields.has('salary')).toBe(false); + }); +}); + +describe('assertReadableQueryFields', () => { + it('rejects a where predicate on a hidden field (the oracle)', () => { + expect(() => + assertReadableQueryFields({ where: { salary: { $gte: 100000 } } }, HIDDEN_SALARY, 'employee'), + ).toThrowError(/salary/); + }); + + it('rejects sorting by a hidden field and reports it as a 403 sentinel', () => { + try { + assertReadableQueryFields({ orderBy: [{ field: 'salary', order: 'desc' }] }, HIDDEN_SALARY, 'employee'); + expect.unreachable('should have thrown'); + } catch (e) { + expect(isPermissionDeniedError(e)).toBe(true); + expect((e as { details?: { fields?: string[] } }).details?.fields).toEqual(['salary']); + } + }); + + it('rejects hidden fields buried in $or branches', () => { + expect(() => + assertReadableQueryFields( + { where: { $or: [{ status: 'open' }, { salary: { $gt: 1 } }] } }, + HIDDEN_SALARY, + 'employee', + ), + ).toThrow(); + }); + + it('passes queries touching only readable fields', () => { + expect(() => + assertReadableQueryFields( + { where: { status: 'open' }, orderBy: [{ field: 'due_date', order: 'asc' }] }, + HIDDEN_SALARY, + 'employee', + ), + ).not.toThrow(); + }); + + it('passes when field permissions grant read (readable !== false)', () => { + expect(() => + assertReadableQueryFields( + { where: { salary: { $gte: 1 } } }, + { salary: { readable: true } }, + 'employee', + ), + ).not.toThrow(); + }); + + it('no-ops when no field permissions are configured', () => { + expect(() => assertReadableQueryFields({ where: { salary: 1 } }, {}, 'employee')).not.toThrow(); + }); +}); diff --git a/packages/plugins/plugin-security/src/predicate-guard.ts b/packages/plugins/plugin-security/src/predicate-guard.ts new file mode 100644 index 0000000000..3af48b422e --- /dev/null +++ b/packages/plugins/plugin-security/src/predicate-guard.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Field-level predicate guard — anti filter-oracle (objectui#2251). + * + * FieldMasker strips unreadable fields from query RESULTS, but a caller can + * still probe a hidden field's VALUE through predicates: filtering + * `salary >= 100000` (or sorting / grouping by `salary`) changes which rows + * come back even though the column itself is masked — presence/absence is + * the oracle. The objectui `/data` surface (ADR-0055) makes arbitrary + * URL-driven filters a first-class citizen, so this hole must be closed at + * the engine, independent of anything the client sends. + * + * Policy: REJECT (403), never silently rewrite. Dropping predicates changes + * query semantics unpredictably (removing an `$or` branch narrows results; + * removing an `$and` branch widens them — the widening direction re-opens + * the oracle). Salesforce FLS behaves the same way: querying a hidden field + * is an error, not a silent no-op. The error message carries the offending + * field names so authors can fix the query. + * + * Ordering contract: this guard MUST run against the CALLER-supplied AST, + * before RLS filter injection — RLS policies legitimately reference fields + * the caller cannot read (e.g. `owner_id`), and must not be rejected. + */ + +import { PermissionDeniedError } from './errors.js'; + +interface FieldPermissionLike { + readable?: boolean; +} + +/** Logical keys of the FilterCondition grammar — never field names. */ +const LOGICAL_KEYS = new Set(['$and', '$or', '$not']); + +/** + * Collect every field name referenced by a FilterCondition. Dotted paths + * and nested-relation conditions gate on their FIRST segment / top-level + * relation field — local field permissions govern local traversal. + */ +export function collectConditionFields(condition: unknown, out: Set = new Set()): Set { + if (!condition || typeof condition !== 'object' || Array.isArray(condition)) return out; + for (const [key, value] of Object.entries(condition as Record)) { + if (LOGICAL_KEYS.has(key)) { + if (Array.isArray(value)) for (const sub of value) collectConditionFields(sub, out); + else collectConditionFields(value, out); + continue; + } + out.add(key.split('.')[0]); + } + return out; +} + +/** + * Collect every field referenced by the query's row-shaping clauses: + * where / orderBy / groupBy / having / aggregations (field + FILTER) / + * window functions (field + partitionBy + over.orderBy). `fields` + * (projection) is intentionally NOT collected — selecting a hidden field is + * harmless because FieldMasker strips it from the result; only predicates + * leak. + */ +export function collectQueryFields(ast: Record): Set { + const out = new Set(); + collectConditionFields(ast.where, out); + collectConditionFields(ast.having, out); + + const orderBy = ast.orderBy; + if (Array.isArray(orderBy)) { + for (const s of orderBy) { + const field = (s as { field?: unknown })?.field; + if (typeof field === 'string') out.add(field.split('.')[0]); + } + } + + const groupBy = ast.groupBy; + if (Array.isArray(groupBy)) { + for (const g of groupBy) { + if (typeof g === 'string') out.add(g.split('.')[0]); + else if (g && typeof g === 'object' && typeof (g as { field?: unknown }).field === 'string') { + out.add(((g as { field: string }).field).split('.')[0]); + } + } + } + + const aggregations = ast.aggregations; + if (Array.isArray(aggregations)) { + for (const a of aggregations) { + const field = (a as { field?: unknown })?.field; + if (typeof field === 'string' && field !== '*') out.add(field.split('.')[0]); + collectConditionFields((a as { filter?: unknown })?.filter, out); + } + } + + const windowFunctions = ast.windowFunctions; + if (Array.isArray(windowFunctions)) { + for (const w of windowFunctions) { + const field = (w as { field?: unknown })?.field; + if (typeof field === 'string') out.add(field.split('.')[0]); + const over = (w as { over?: { partitionBy?: unknown; orderBy?: unknown } })?.over; + if (Array.isArray(over?.partitionBy)) { + for (const p of over.partitionBy) if (typeof p === 'string') out.add(p.split('.')[0]); + } + if (Array.isArray(over?.orderBy)) { + for (const s of over.orderBy) { + const f = (s as { field?: unknown })?.field; + if (typeof f === 'string') out.add(f.split('.')[0]); + } + } + } + } + + return out; +} + +/** + * Throw PermissionDeniedError (→ HTTP 403) when the caller-supplied query + * references a field its field-level permissions mark non-readable. + */ +export function assertReadableQueryFields( + ast: Record, + fieldPermissions: Record, + object: string, +): void { + const hidden = new Set( + Object.entries(fieldPermissions) + .filter(([, perm]) => perm && perm.readable === false) + .map(([field]) => field), + ); + if (hidden.size === 0) return; + + const offending = [...collectQueryFields(ast)].filter((f) => hidden.has(f)); + if (offending.length === 0) return; + + throw new PermissionDeniedError( + `[Security] Access denied: query on '${object}' references field(s) not readable by the caller: ` + + `${offending.join(', ')}. Filtering, sorting, grouping, or aggregating by a hidden field ` + + `would leak its values (filter oracle) — remove these predicates or grant field read access.`, + { object, fields: offending, reason: 'field_predicate_denied' }, + ); +} diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index ab56f9994a..7605f0787a 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -10,6 +10,7 @@ import { bootstrapSystemCapabilities } from './bootstrap-system-capabilities.js' import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; import { matchesFilterCondition } from '@objectstack/formula'; import { FieldMasker } from './field-masker.js'; +import { assertReadableQueryFields } from './predicate-guard.js'; import { PermissionDeniedError } from './errors.js'; import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js'; import { @@ -682,6 +683,23 @@ export class SecurityPlugin implements Plugin { } } + // 2.9. Field-level predicate guard (anti filter-oracle, objectui#2251). + // FieldMasker (step 4) only strips hidden fields from RESULTS — a + // caller could still probe a hidden field's value by filtering / + // sorting / grouping on it (row presence is the oracle; the objectui + // /data surface makes URL-driven predicates first-class). Reject such + // queries outright — silent predicate dropping would change query + // semantics unpredictably. MUST run against the CALLER's AST, before + // the RLS injection below: RLS policies legitimately reference fields + // the caller cannot read (e.g. owner_id). + if (opCtx.ast) { + let guardPerms = this.permissionEvaluator.getFieldPermissions(opCtx.object, permissionSets); + guardPerms = this.foldFieldRequiredPermissions(guardPerms, secMeta.fieldRequiredPermissions, permissionSets); + if (Object.keys(guardPerms).length > 0) { + assertReadableQueryFields(opCtx.ast as unknown as Record, guardPerms, opCtx.object); + } + } + // 3. RLS filter injection. The policy collection + field-existence // safety + compile (incl. the fail-closed deny sentinel) is shared with // the public getReadFilter service via computeRlsFilter, so the engine diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index 77cc53a352..9a007faa51 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -280,6 +280,53 @@ describe('NavigationItemSchema (Recursive)', () => { expect(() => NavigationItemSchema.parse(navItem)).not.toThrow(); }); + + it('accepts a filters-only object item (bare data surface slice)', () => { + const item = { + id: 'nav_my_open', + label: 'My Open', + type: 'object' as const, + objectName: 'ticket', + filters: { owner_id: '{current_user_id}', status: 'open' }, + }; + expect(() => NavigationItemSchema.parse(item)).not.toThrow(); + }); + + it('rejects filters combined with viewName (ambiguous landing)', () => { + const item = { + id: 'nav_bad', + label: 'Bad', + type: 'object' as const, + objectName: 'ticket', + viewName: 'by_status', + filters: { status: 'open' }, + }; + expect(() => NavigationItemSchema.parse(item)).toThrow(); + }); + + it('rejects filters combined with recordId (ambiguous landing)', () => { + const item = { + id: 'nav_bad2', + label: 'Bad', + type: 'object' as const, + objectName: 'ticket', + recordId: '{current_user_id}', + filters: { status: 'open' }, + }; + expect(() => NavigationItemSchema.parse(item)).toThrow(); + }); + + it('tolerates the legacy recordId + viewName combination (documented: viewName ignored)', () => { + const item = { + id: 'nav_legacy', + label: 'Legacy', + type: 'object' as const, + objectName: 'sys_user', + recordId: '{current_user_id}', + viewName: 'all', + }; + expect(() => NavigationItemSchema.parse(item)).not.toThrow(); + }); }); describe('AppSchema', () => { diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index df3b8054c6..c513cedb12 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -141,12 +141,42 @@ export const ObjectNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ * links); a slice worth curating and reusing belongs in a named view * via `viewName`. Values support the same template variables as * `recordId`. Precedence: `recordId` → `filters` → `viewName`. + * + * Mutually exclusive with `recordId` / `viewName` — enforced by + * {@link NavigationItemSchema} (see `objectNavTargetExclusivity`) so the + * ambiguous combination is unrepresentable rather than silently resolved + * by precedence. */ filters: z.record(z.string(), z.string()).optional().describe( - 'URL filter conditions — targets the /:objectName/data bare surface via filter[]= params instead of a saved view. Values support template vars {current_user_id}, {current_org_id}. Precedence: recordId → filters → viewName.', + 'URL filter conditions — targets the /:objectName/data bare surface via filter[]= params instead of a saved view. Values support template vars {current_user_id}, {current_org_id}. Mutually exclusive with recordId/viewName.', ), })); +/** + * Correct-by-construction guard (ADR-0053 philosophy): `filters` combined + * with `recordId` or `viewName` is an authoring ambiguity — runtime + * precedence would silently ignore one of them (a stale `recordId` hijacks + * a configured `filters` slice). Reject it at validation with the fix in + * the message. The legacy `recordId` + `viewName` combination stays + * tolerated: it predates this guard and is documented as "viewName is + * ignored when recordId is set". + */ +const objectNavTargetExclusivity = ( + item: { filters?: unknown; recordId?: unknown; viewName?: unknown }, + ctx: z.RefinementCtx, +): void => { + if (item.filters && (item.recordId || item.viewName)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['filters'], + message: + '`filters` cannot be combined with `recordId` or `viewName` — pick ONE landing: ' + + 'recordId (record deep-link), filters (/data slice), or viewName (named view). ' + + 'Remove the extra field(s); runtime precedence would silently ignore them.', + }); + } +}; + /** * 2. Dashboard Navigation Item * Navigates to a specific dashboard. @@ -241,7 +271,7 @@ export const NavigationItemSchema: z.ZodType = z.lazy(() => z.union([ ObjectNavItemSchema.extend({ children: z.array(NavigationItemSchema).optional().describe('Child navigation items (e.g. specific views)'), - }), + }).superRefine(objectNavTargetExclusivity), DashboardNavItemSchema, PageNavItemSchema, UrlNavItemSchema, From b67c008417c4fcc2014da4622ddc36e31dd27498 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 08:36:06 +0000 Subject: [PATCH 2/2] docs(permissions): document the field-predicate 403 in the authorization layer table Layer 6 (field-level security) now states the predicate guard: caller queries filtering/sorting/grouping/aggregating by a non-readable field are rejected (403 field_predicate_denied) rather than value-leaking through row presence; RLS-injected predicates are exempt. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018m3GX7EMKNPDZuee152EKK --- content/docs/permissions/authorization.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/authorization.mdx b/content/docs/permissions/authorization.mdx index dc5654752e..b45502d261 100644 --- a/content/docs/permissions/authorization.mdx +++ b/content/docs/permissions/authorization.mdx @@ -47,7 +47,7 @@ site — the file you read when behavior surprises you. | 3 | **Object CRUD** | `allowRead/Create/Edit/Delete` (+ the destructive lifecycle class `allowTransfer/Restore/Purge`, gated ahead of the M2 operations — #1883) resolved across the caller's permission sets. | `packages/plugins/plugin-security/src/permission-evaluator.ts` `checkObjectPermission` | fail-closed 403 | | 4 | **OWD / sharing** | Org-wide default (`private` / `public_read` / `public_read_write` / `controlled_by_parent`), manual record shares, criteria sharing rules (owner-type rules are declared but seed-skipped — [not enforced](/docs/permissions/sharing-rules#owner-based-sharing-rules)), business-unit hierarchy widening (ADR-0057 D5: scope-depth hierarchy lives on `sys_business_unit`, not roles). | `packages/plugins/plugin-sharing/src/sharing-service.ts` + `sharing-rule-service.ts` | owner-only baseline | | 5 | **Row-level security** | CEL predicates (`using` read filter, `check` write post-image) compiled into the query. If **no** applicable policy compiles, the result is a deny-all sentinel (fail-closed); an uncompilable policy alongside compilable ones is excluded from the OR-union **with a logged warning** — exclusion can only narrow access, never widen it. Tenant isolation is a wildcard RLS rule AND-ed on top. | `packages/plugins/plugin-security/src/rls-compiler.ts` + `security-plugin.ts` | fail-closed | -| 6 | **Field-level security** | Read mask (strip non-readable fields) + write deny per `fields` rules. | `packages/plugins/plugin-security/src/field-masker.ts` | see posture note below | +| 6 | **Field-level security** | Read mask (strip non-readable fields) + write deny per `fields` rules. Caller queries that **filter, sort, group, or aggregate by** a non-readable field are rejected outright (HTTP 403, `field_predicate_denied`) — masking only the output would leave row presence as a value oracle. RLS-injected predicates are exempt (they run after the guard and may reference hidden fields like `owner_id`). | `packages/plugins/plugin-security/src/field-masker.ts` + `predicate-guard.ts` | fail-closed on predicates; see posture note below | Two orthogonal identity-layer gates run before all of this: the ADR-0069 **authentication-policy gate** (password expiry / enforced MFA blocks a gated