Skip to content

Commit 2b176bd

Browse files
committed
fix(spec): isComposedReadableIds refuses a value that also carries the uncomposed brand, and the scope wording says non-null, non-array object (contract review A1, A2)
A hand-built value carrying both global-registry brands answered true to both recognisers, so the docblock's "no value answers true to both" was false. It now reads as an uncomposed leaf: isComposedReadableIds answers false and isReadableIds (unchanged) answers true. A pin holds the double-branded value and the comparand door's measured verdict on it. "Plain" is replaced by what isComposableScope checks, in readable-ids.ts and in the changeset. Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ Co-authored-by: Claude <noreply@anthropic.com>
1 parent e7221cb commit 2b176bd

3 files changed

Lines changed: 30 additions & 9 deletions

File tree

‎.changeset/23009-rls-deny-sentinel-composed-readable-ids.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ Clause-②: yes (widening: new exported members of the published spec `data` ent
99

1010
- **`RLS_DENY_FILTER` moves into the spec.** FROM `@objectstack/plugin-security`'s own frozen literal TO `RLS_DENY_FILTER` exported by `@objectstack/spec/data`. The value is unchanged: `{ id: '__rls_deny__:00000000-0000-0000-0000-000000000000' }`, an `id` equality no record satisfies, so a consumer that never heard of it still reads zero rows. `@objectstack/plugin-security` keeps exporting `RLS_DENY_FILTER` under the same name and type; it is now the spec's object, not a second copy. Nothing to change in code that imports it from either package.
1111
- **One object, compared by identity.** `getReadFilter` (`ISecurityService`) now returns the sentinel itself, not a copy, for a principal-less caller, a sharing scope that cannot be resolved, an on-behalf-of caller, a caller with no permission set and a resolution failure. It also returns the sentinel itself when the only layer that constrains is the tenant wall's deny, the RLS compiler's deny (every applicable policy dropped), or the deny of a `controlled_by_parent` object whose master cannot be resolved. Before this release most of those returned a spread copy, which matched the same zero rows but was a different object. Not every zero-row answer is the sentinel: a deny ANDed with another layer is returned as the composed `$and`, with the sentinel inside it, and the record-sharing layer's own deny, `{ id: '__deny_all__' }`, is returned unchanged when sharing is the only layer that constrains. ADR-0139 D2 step 2 has the engine's `ReadableIds` composition recognise the deny by `scope === RLS_DENY_FILTER` and answer it `$in: []`; any other zero-row answer composes like any scope and still matches no row. The object is frozen: code that edited the filter `getReadFilter` returned now gets a `TypeError` instead of silently changing a deny.
12-
- **The composed leaf.** `ReadableIds.compose(leaf, scope)` builds the frozen value `{ object, scope }` the engine hands a driver in place of a `ReadableIds` leaf (D2 step 3). It is branded under `COMPOSED_READABLE_IDS_BRAND` (`Symbol.for('objectstack.filter.readableIds.composed')`), so a JSON round-trip, a structured clone or a spread drops the brand. `isComposedReadableIds(value)` recognises it. The type `ComposedReadableIds` is exported. `scope` is `undefined` for "no row restriction"; `compose` throws a `TypeError` for anything but a `ReadableIds.of` leaf, for a scope that is not a plain filter object, and for `RLS_DENY_FILTER` itself, which D2 step 2 answers `$in: []`.
12+
- **The composed leaf.** `ReadableIds.compose(leaf, scope)` builds the frozen value `{ object, scope }` the engine hands a driver in place of a `ReadableIds` leaf (D2 step 3). It is branded under `COMPOSED_READABLE_IDS_BRAND` (`Symbol.for('objectstack.filter.readableIds.composed')`), so a JSON round-trip, a structured clone or a spread drops the brand. `isComposedReadableIds(value)` recognises it. The type `ComposedReadableIds` is exported. `scope` is `undefined` for "no row restriction"; `compose` throws a `TypeError` for anything but a `ReadableIds.of` leaf, for a scope that is neither `undefined` nor a non-null, non-array object, and for `RLS_DENY_FILTER` itself, which D2 step 2 answers `$in: []`.
1313
- **`isReadableIds` answers as before.** It answers `false` for a composed leaf, so it still means "an uncomposed leaf", and a driver refuses on it. The comparand-shape door's `admitReadableIds` admits the uncomposed leaf only: a composed leaf handed to the engine from outside is refused `INVALID_FILTER` / 400 as a non-list comparand.

‎packages/spec/src/data/readable-ids.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,21 @@ describe('isComposedReadableIds — disjoint from isReadableIds, and every copy
268268
expect(isComposedReadableIds(leaf)).toBe(false);
269269
});
270270

271+
it('a hand-built value carrying BOTH brands is not composed: it reads as an uncomposed leaf, whichever recogniser is asked first', () => {
272+
// Both keys are global-registry symbols, so in-process code can build this.
273+
const both = { object: 'x', scope: undefined };
274+
Object.defineProperty(both, READABLE_IDS_BRAND, { value: true });
275+
Object.defineProperty(both, COMPOSED_READABLE_IDS_BRAND, { value: true });
276+
expect(isComposedReadableIds(both)).toBe(false);
277+
expect(isReadableIds(both)).toBe(true);
278+
// The comparand door, as measured: it admits the value as an UNCOMPOSED leaf
279+
// for the composing caller (so the engine composes it from the caller's
280+
// access) and refuses it for every other caller.
281+
expect(() => assertListComparandShapes({ record_id: { $in: both } }, undefined, 'where', ADMIT)).not.toThrow();
282+
const err = refusalOf(() => assertListComparandShapes({ record_id: { $in: both } }));
283+
expect(envelopeOf(err)).toEqual(ENVELOPE);
284+
});
285+
271286
it('a JSON round-trip, a structured clone, a spread and Object.assign all drop the brand', () => {
272287
const copies = {
273288
json: JSON.parse(JSON.stringify(composed)),

‎packages/spec/src/data/readable-ids.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -200,25 +200,31 @@ export interface ComposedReadableIds {
200200
/**
201201
* Is `value` a branded {@link ComposedReadableIds} leaf?
202202
*
203-
* Own symbol key only, and only the declared value `true`; `object` a
204-
* non-empty string; `scope` an own key holding `undefined` or a plain,
205-
* non-array object. Every copy that lost the brand, every hand-built shape that
206-
* breaks one of those, an uncomposed {@link ReadableIds} leaf, `null` and every
207-
* primitive answer `false`.
203+
* Own symbol key only, and only the declared value `true`; no own
204+
* {@link READABLE_IDS_BRAND} beside it; `object` a non-empty string; `scope`
205+
* an own key holding `undefined` or a non-null, non-array object. Every copy
206+
* that lost the brand, every hand-built shape that breaks one of those, an
207+
* uncomposed {@link ReadableIds} leaf, `null` and every primitive answer
208+
* `false`.
208209
*
209-
* Disjoint from {@link isReadableIds}: no value answers `true` to both.
210+
* Disjoint from {@link isReadableIds}: no value answers `true` to both. A
211+
* value that carries both brands answers `false` here, so it is never a
212+
* composed leaf: when its uncomposed brand is valid, {@link isReadableIds}
213+
* answers `true` and the value is refused as an uncomposed leaf, whichever of
214+
* the two recognisers is asked first.
210215
*/
211216
export function isComposedReadableIds(value: unknown): value is ComposedReadableIds {
212217
if (value === null || typeof value !== 'object' || Array.isArray(value)) return false;
213218
if (!Object.prototype.hasOwnProperty.call(value, COMPOSED_READABLE_IDS_BRAND)) return false;
214219
if ((value as Record<symbol, unknown>)[COMPOSED_READABLE_IDS_BRAND] !== true) return false;
220+
if (Object.prototype.hasOwnProperty.call(value, READABLE_IDS_BRAND)) return false;
215221
const object = (value as { object?: unknown }).object;
216222
if (typeof object !== 'string' || object.length === 0) return false;
217223
if (!Object.prototype.hasOwnProperty.call(value, 'scope')) return false;
218224
return isComposableScope((value as { scope?: unknown }).scope);
219225
}
220226

221-
/** `undefined` (no restriction) or a plain, non-array object — the scope's two declared spellings. */
227+
/** `undefined` (no restriction) or a non-null, non-array object — the scope's two declared spellings. */
222228
function isComposableScope(scope: unknown): boolean {
223229
return scope === undefined || (scope !== null && typeof scope === 'object' && !Array.isArray(scope));
224230
}
@@ -261,7 +267,7 @@ export const ReadableIds: {
261267
* @param scope the parent's lowered read scope, or `undefined` for "no row
262268
* restriction" (`getReadFilter`'s own spelling of it).
263269
* @throws TypeError when `leaf` is not a branded {@link ReadableIds} leaf;
264-
* when `scope` is neither `undefined` nor a plain, non-array object;
270+
* when `scope` is neither `undefined` nor a non-null, non-array object;
265271
* or when `scope` IS {@link RLS_DENY_FILTER}, which D2 step 2
266272
* answers `$in: []` and never composes.
267273
*/

0 commit comments

Comments
 (0)