Skip to content

Commit 1850ebb

Browse files
os-zhuangclaude
andauthored
docs(spec): correct the reuse-safety claim on the filter-subtree mark, and pin the invariant it rests on (#9073)
The mark's docblock called itself "safe on filter objects that are reused across requests (view metadata, cached scopes)". The #8794 survey measured that claim: it holds only where provenance is INTRINSIC to the subtree. A 'policy' scope is intrinsically policy, so a stale mark is still correct and can only withhold. A caller's `where` is contextual — a stale 'author' mark discloses under a root the next request never vouched, and first-mark-wins makes the corrective 'policy' stamp a silent no-op. Corrects both docblocks (the mark's, and `getReadFilter`'s — the public method whose documented use puts a platform predicate in the vouched slot), and adds four pin tests naming the invariant verbatim: no filter object that can be vouched 'author' may outlive the request that vouched it. No mechanism change: first-mark-wins, the non-writable symbol and positional resolution are untouched, and the accepted metadata set is unmoved. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8640fb2 commit 1850ebb

3 files changed

Lines changed: 190 additions & 3 deletions

File tree

packages/spec/src/contracts/security-service.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,27 @@ export interface ISecurityService {
211211
* that cannot compute the delegator intersection, returns a DENY filter that
212212
* matches zero rows — never `undefined`. `undefined` means one thing only:
213213
* this caller has no row restriction on this object.
214+
*
215+
* **⚠️ Request-scoped: call it per request, and never memoise what it
216+
* returns.** The documented use — `engine.find(object, { where: await
217+
* security.getReadFilter(object, ctx) })` — puts a PLATFORM-authored
218+
* predicate into the `options.where` slot, and that slot is exactly what the
219+
* read-scope merge boundaries vouch as the CALLER's own predicate (the
220+
* `'author'` mark of `../data/filter-subtree-provenance.js`). Two
221+
* consequences, neither of which a host may design around:
222+
*
223+
* - the vouch is consumed as *"safe to disclose to this caller"*, not as
224+
* *"this caller passed it"*. A policy predicate sitting in that slot can
225+
* therefore carry its own `{ $field }` operands into a refusal message
226+
* that the #7929 redaction exists to withhold;
227+
* - the mark is permanent — first mark wins, and it is non-writable — so a
228+
* filter cached across requests keeps the FIRST request's vouch, and no
229+
* later boundary can correct it. The invariant this rests on is that no
230+
* filter object which can be vouched `'author'` outlives the request that
231+
* vouched it (#8794 / #8836).
232+
*
233+
* A host that must scope repeated queries re-calls this method; it does not
234+
* hold the returned object.
214235
*/
215236
getReadFilter(object: string, context?: SecurityContext): Promise<FilterCondition | undefined>;
216237

packages/spec/src/data/filter-subtree-provenance.test.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
* frozen subtree, a serialized round-trip, a corrupted mark value, a rewritten
1010
* tree, an aliased node under conflicting arms — must land on `null`, never on
1111
* `'author'`.
12+
*
13+
* [#8836] The last block pins the one shape that does NOT land there — a
14+
* vouchable filter object reused across requests — and the caller-side
15+
* invariant that keeps it out of reach. Grep for "may outlive the request".
1216
*/
1317

1418
import { describe, it, expect } from 'vitest';
@@ -158,3 +162,129 @@ describe('resolveFilterSubtreeProvenance', () => {
158162
expect(resolveFilterSubtreeProvenance('root' as never, {})).toBe(null);
159163
});
160164
});
165+
166+
/**
167+
* [#8836, from the #8794 survey] The invariant the fail-closed direction
168+
* silently depends on, made executable:
169+
*
170+
* > no filter object that can be vouched `'author'` may outlive the request
171+
* > that vouched it.
172+
*
173+
* ⚠️ **These tests assert what IS, not what SHOULD BE.** The mark's mechanism
174+
* is #8220's and is deliberately untouched here: first-mark-wins, the
175+
* non-writable symbol, and positional resolution all stay exactly as declared.
176+
* What this block pins is the COST of breaking the invariant — the disclosure a
177+
* reused vouchable object produces, measured against the control that makes the
178+
* measurement mean something. `packages/spec` cannot enforce the invariant
179+
* itself: "the request" is not a concept this module can see, and the callers
180+
* that must honour it live in `plugin-security` / `plugin-sharing`
181+
* (`markFilterSubtreeProvenance(…, 'author')` is reachable only on
182+
* `options.where` itself or on the arms of a pure `$and` root). So the guard
183+
* this block can offer is that the consequence stays visible and stays
184+
* asserted: a future change that makes any expectation below go red is a
185+
* change to the mark's mechanism, which #8794's ruling routes to a spec-seat
186+
* ruling BEFORE implementation — not something to fix by editing these
187+
* numbers.
188+
*/
189+
describe("invariant: no filter object that can be vouched 'author' may outlive the request that vouched it", () => {
190+
/**
191+
* The identity vouch both read-scope merge boundaries perform, reduced to the
192+
* decision that matters: `options.where` is stamped `'author'` only while the
193+
* AST still holds that exact object. Written out rather than imported because
194+
* `packages/spec` sits UNDER both plugins — the real sites are
195+
* `plugin-security`'s CRUD injection and `plugin-sharing`'s read-filter merge,
196+
* and the survey measured both. Nothing here is a stand-in for behaviour under
197+
* test; the subject is this module's exports.
198+
*/
199+
const vouchCallerWhere = (ast: { where: unknown }, options: { where: object }): void => {
200+
if (ast.where !== options.where) return; // re-shaped between the two: no vouch
201+
markFilterSubtreeProvenance(options.where, 'author');
202+
};
203+
204+
/**
205+
* One request. `composeScope` is the ordinary case where a sibling middleware
206+
* (or ADR-0061 search expansion) ANDs its own predicate into the AST BEFORE
207+
* the vouch runs, so this request's boundary no longer recognises the caller's
208+
* object and vouches nothing at all.
209+
*/
210+
const runRequest = (callerWhere: object, opts: { composeScope?: boolean } = {}) => {
211+
const options = { where: callerWhere };
212+
const ast: { where: unknown } = { where: options.where };
213+
if (opts.composeScope) {
214+
ast.where = {
215+
$and: [callerWhere, markFilterSubtreeProvenance({ organization_id: 'org_1' }, 'policy')],
216+
};
217+
}
218+
vouchCallerWhere(ast, options);
219+
return ast;
220+
};
221+
222+
it('a vouched filter that OUTLIVES its request discloses under a root the next request never vouched', () => {
223+
// A module-level constant, a cached scope, view metadata: one object, two requests.
224+
const reusedWhere = { amount: { $gt: { $field: 'budget' } } };
225+
226+
// Request 1 — identity holds, so this request's boundary vouches it.
227+
runRequest(reusedWhere);
228+
expect(filterSubtreeProvenanceOf(reusedWhere)).toBe('author');
229+
230+
// Request 2 — a sibling middleware composed first, so this boundary vouches NOTHING.
231+
const ast = runRequest(reusedWhere, { composeScope: true });
232+
expect(filterSubtreeProvenanceOf(ast.where)).toBe(null);
233+
234+
// …and the caller's comparand still answers 'author' — request 1's vouch,
235+
// spent as a licence to disclose inside request 2. This is the whole cost.
236+
expect(resolveFilterSubtreeProvenance(ast.where, reusedWhere.amount.$gt)).toBe('author');
237+
});
238+
239+
it('CONTROL: the same shape built FRESH per request resolves null in that same position', () => {
240+
const buildWhere = () => ({ amount: { $gt: { $field: 'budget' } } });
241+
242+
const first = buildWhere();
243+
runRequest(first);
244+
expect(filterSubtreeProvenanceOf(first)).toBe('author'); // the vouch still happens
245+
246+
// Structurally identical, same position, same middleware — only the object's
247+
// lifetime differs, and the verdict flips to withheld. That difference is
248+
// the invariant, and it is the reason the test above is not a tautology.
249+
const second = buildWhere();
250+
const ast = runRequest(second, { composeScope: true });
251+
expect(resolveFilterSubtreeProvenance(ast.where, second.amount.$gt)).toBe(null);
252+
});
253+
254+
it('a later boundary cannot repair a stale author mark — the corrective policy stamp is a silent no-op', () => {
255+
// The mirror of the "first mark wins" case above, and the load-bearing one:
256+
// there a stale 'policy' mark resisted an 'author' overwrite (fail-closed);
257+
// here a stale 'author' mark resists the 'policy' correction (fail-open).
258+
const reused = markFilterSubtreeProvenance({ owner: { $field: 'manager_id' } }, 'author');
259+
260+
markFilterSubtreeProvenance(reused, 'policy');
261+
expect(filterSubtreeProvenanceOf(reused)).toBe('author');
262+
263+
// Nor can a boundary force it: the mark is non-writable and non-configurable.
264+
expect(() =>
265+
Object.defineProperty(reused, FILTER_SUBTREE_PROVENANCE, { value: 'policy' }),
266+
).toThrow(TypeError);
267+
expect(filterSubtreeProvenanceOf(reused)).toBe('author');
268+
});
269+
270+
it('the asymmetry: reuse degrades a policy mark toward WITHHOLDING and an author mark toward DISCLOSING', () => {
271+
// Why "the classification of one subtree does not change between requests"
272+
// is sound for a policy scope and unsound for a caller's `where`: provenance
273+
// is intrinsic to the first and contextual to the second. Same reuse, same
274+
// unvouched root, opposite fail direction.
275+
const unvouchedRoot = (arm: object) => ({ $and: [{ stage: 'won' }, arm] });
276+
277+
const staleScope = markFilterSubtreeProvenance(
278+
{ organization_id: { $field: 'ctx_org' } },
279+
'policy',
280+
);
281+
expect(
282+
resolveFilterSubtreeProvenance(unvouchedRoot(staleScope), staleScope.organization_id),
283+
).toBe('policy'); // still correct next request, and it can only withhold
284+
285+
const staleWhere = markFilterSubtreeProvenance({ amount: { $gt: { $field: 'budget' } } }, 'author');
286+
expect(resolveFilterSubtreeProvenance(unvouchedRoot(staleWhere), staleWhere.amount.$gt)).toBe(
287+
'author',
288+
); // no longer this request's caller, and it DISCLOSES
289+
});
290+
});

packages/spec/src/data/filter-subtree-provenance.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,45 @@ export const FILTER_SUBTREE_PROVENANCE: symbol = Symbol.for(
120120
* unchanged: the actor closest to the subtree's creation is the one that knows
121121
* its provenance, and letting a later, more distant actor overwrite that would
122122
* let a generic boundary re-vouch a policy predicate as the caller's. Marking
123-
* is therefore idempotent, and safe on filter objects that are reused across
124-
* requests (view metadata, cached scopes): the classification of one subtree
125-
* does not change between requests.
123+
* is therefore idempotent.
124+
*
125+
* ## ⚠️ Idempotent is NOT "safe to reuse across requests"
126+
*
127+
* This paragraph used to end by calling the mark *"safe on filter objects that
128+
* are reused across requests (view metadata, cached scopes): the
129+
* classification of one subtree does not change between requests"*. The
130+
* #8794 survey measured that claim and it is true in ONE direction only; the
131+
* correction is pinned by `filter-subtree-provenance.test.ts` (#8836). The
132+
* claim holds where provenance is **intrinsic** to the subtree, and a caller's
133+
* `where` is the half where it is not:
134+
*
135+
* - a `'policy'` scope IS intrinsically policy — the platform's predicate in
136+
* every request — so a stale `'policy'` mark is still correct next request,
137+
* and first-mark-wins is what stops a distant boundary re-vouching it. This
138+
* is the direction the rule was written for, and it degrades toward
139+
* WITHHOLDING;
140+
* - a caller's `where` is **contextual** — the same object is the caller's own
141+
* predicate in one request and not in another. A stale `'author'` mark
142+
* degrades toward DISCLOSING, and because first-mark-wins is absolute the
143+
* corrective `'policy'` stamp a later boundary would apply is a silent
144+
* no-op — the already-marked guard below returns the subtree unchanged.
145+
* Nothing downstream can repair it either: the mark is non-writable and
146+
* non-configurable.
147+
*
148+
* So "the classification does not change between requests" is an assumption
149+
* about the CALLER POPULATION, not a property of this mark. The invariant the
150+
* design actually rests on is:
151+
*
152+
* > no filter object that can be vouched `'author'` may outlive the request
153+
* > that vouched it.
154+
*
155+
* It holds across every in-repo caller today — enumerated with controls in
156+
* #8794 — and it holds *incidentally*: every caller in a markable position
157+
* (`options.where` itself, or an arm of a pure `$and` root) happens to build
158+
* its filter fresh per request. This module does not and cannot enforce it,
159+
* because "the request" is not a concept it can see. What is enforced is that
160+
* the cost of breaking it stays visible: the pin test asserts the disclosure a
161+
* reused vouchable object produces, and its fresh-object control.
126162
*
127163
* Fail-closed by silence: a non-object subtree, a frozen/sealed one, or any
128164
* `defineProperty` refusal leaves the subtree unmarked — and unmarked is

0 commit comments

Comments
 (0)