diff --git a/.changeset/oauth-agent-runs-as-the-user.md b/.changeset/oauth-agent-runs-as-the-user.md new file mode 100644 index 0000000000..4c945367bb --- /dev/null +++ b/.changeset/oauth-agent-runs-as-the-user.md @@ -0,0 +1,40 @@ +--- +"@objectstack/plugin-security": minor +"@objectstack/spec": minor +"@objectstack/mcp": minor +"@objectstack/runtime": minor +--- + +fix(security): an OAuth-connected MCP agent runs at its delegator's record depth — "you connect as yourself" becomes true (#16549) + +Maintainer ruling, decision batch #81 item 1 (2026-09-08), option 1: **the OAuth agent runs with the user's own permissions; the ceiling only subtracts; the diagnostic lands regardless.** + +**The defect, measured.** The Setup → Connect an Agent page promises, verbatim, *"you connect as yourself, and every call runs under your own permissions and row-level security."* It did not. The same sales manager, same questions, same server: + +| identity path | `crm_account` | `crm_opportunity` | `crm_task` | +|:--|--:|--:|--:| +| API key, `principalKind: human` | 9 | 23 | 45 | +| OAuth, `principalKind: agent`, `onBehalfOf` = same user | **5** | **0** | **0** | + +The agent read `own` scope where the human read `viewAllRecords`, so any profile whose visibility comes from `viewAllRecords` — every manager-type profile — collapsed to *own + explicit shares*. And it was **silent**: the MCP tools answered `total: 0` with no note, so the agent reported "there are no opportunities this quarter" as a fact about the data. + +**The mechanism, in one line.** `mcp_agent_data_read` / `mcp_agent_data_write` are pure CAPABILITY ceilings — a `'*'` grant with no `readScope` and no `viewAllRecords`, whose own doc says *"NO row-level security … all row/owner/tenant narrowing comes from the delegating user"*. `PermissionEvaluator.getEffectiveScope` nevertheless answered `'own'` for them, because its owner-only default turns a granting-but-silent set into an owner-scoped one. That default is correct for a principal standing on its own and wrong as an input to an intersection: it made the ADR-0090 D10 fold subtract with an opinion nobody declared. + +**(1) Parity.** A new `PermissionEvaluator.getDeclaredScope` answers the depth a set actually *declares*, or `undefined` when every granting set is silent; `intersectDelegatedScope` reads that silence as **no opinion**, so the delegated principal's own leg contributes no owner narrowing and the delegator's depth stands — `agent ∩ user = user` for visibility. A ceiling that *does* declare a depth keeps its full subtractive force. The explain engine's `depth` layer folds through the identical function, so a report cannot describe an intersection the query did not have. + +⛔ **Only visibility depth moved.** Each ceiling's remaining subtractions are now written down explicitly beside the sets themselves (`objects/default-permission-sets.ts`): `data:read` still cannot write, create, delete, export or `allowTransfer`; `data:write` still cannot `allowTransfer` or export, and `sys_*` / better-auth-managed identity tables stay read-only; neither reaches a `private`-posture object nor carries any `systemPermissions`; a dangling delegator still fails CLOSED; and share-MANAGEMENT authority is still not delegated (`hasWriteBypass` → `false`, `resolveWriteScope` → `'own'` for any on-behalf-of context). Putting `viewAllRecords` / `modifyAllRecords` on the ceiling — the ruling's other permitted route — would have granted `allowTransfer` (`MODIFY_ALL_WRITE_KEYS` covers it) and reached `private` objects through the superuser wildcard, both explicitly fenced off, which is why the fix lands on the intersection instead. + +**(2) The diagnostic, independent of (1).** `ISecurityService.describeDelegationNarrowing` (optional) reports whether the agent ceiling narrowed a delegated read, resolved from the same two evaluator calls the CRUD middleware stashes as `__readScope`. `McpDataBridge.diagnoseDelegation` (optional) carries it to the transport, and MCP `query_records` serves a narrowed result with `delegationNarrowed: true` plus a `warning` sentence naming the D10 intersection — the `partial` / `warning` shape `list_objects` already uses. The rows are still served; what is added is the fact the payload could not previously carry: *this count describes the ceiling, not the object.* An un-narrowed read, a non-delegated read, a bridge with no probe and a throwing probe all render exactly what they rendered before. + +**(3)** The Setup page's promise is untouched — it is now true rather than rewritten. + +Purely additive on every published surface: two new optional members, one new exported type (`DelegationNarrowing`), and one new evaluator method. No existing member changed shape, and the only behavioural change is on the delegated path with a ceiling that declares no depth. + +`DelegationNarrowing` is a **discriminated union** on `narrowed`, not one shape with three optional fields, because the two shapes are not symmetric once released: + +| direction, after release | consumer cost | +|:--|:--| +| ship optional fields, later tighten them to required | a compile break | +| ship discriminated, later loosen it (a new union member, or an optional field on the `true` arm) | none | + +The loose shape buys nothing and forecloses the tightening. It also removes the very failure mode the method exists to prevent: `statement` is the sentence an AI consumer renders, so left optional, a consumer that forgets the `narrowed` check silently renders `undefined` — the same silence the table above measures. The five-member scope ladder it reports names the alias that already exists for it, `ObjectAccessScope` (ADR-0057 D1, `@objectstack/spec/security`), rather than minting a second declaration of one ladder; `resolveWriteScope` now names it too, so the union is spelled once instead of three times and no export is added beyond `DelegationNarrowing` itself. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 749d333f33..941eff49b2 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -9,7 +9,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a service self-write, a migration. This page is **the authority** for what that flag actually does. It exists -because the flag is not one concept: it is a single boolean read at **109 +because the flag is not one concept: it is a single boolean read at **110 distinct sites across 20 packages**, and knowing three of those behaviours gives no hint that the other hundred-and-four exist. Every documented app-side bug traced to `isSystem` had the same shape — the metadata was complete and correct, @@ -133,7 +133,7 @@ that silently does not happen. ### 3. Sharing (`plugin-sharing`) -The largest single consumer — **17 of the 109 sites**. +The largest single consumer — **17 of the 110 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| @@ -279,7 +279,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are independent decisions, and a seed loader plausibly wants the first two but not the third. The concept is nevertheless **staying as one boolean**: -- **Shipped semantics.** `isSystem` is a published contract with 109 read sites +- **Shipped semantics.** `isSystem` is a published contract with 110 read sites in 20 packages. Splitting it is a breaking contract change across all of them. (The ruling was taken when the census read 80 sites in 18 packages; the count has grown, which strengthens rather than weakens the argument.) @@ -353,12 +353,12 @@ still holds equal to the census on every pull request: | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — | | — parsed as a declaration | 23 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | -| — parsed as a property **read** | 115 | ✅ | +| — parsed as a property **read** | 116 | ✅ | | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ | | — the remainder: text inside comments and string literals | 358 | — | | Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ | -| Of those reads: reads of `ExecutionContext.isSystem` | **109** | ✅ | -| — behaviour-bearing (rows 1–63 above) | 105 | ✅ | +| Of those reads: reads of `ExecutionContext.isSystem` | **110** | ✅ | +| — behaviour-bearing (rows 1–63 above) | 106 | ✅ | | — carry the flag onward only (rows 64–67 above) | 4 | ✅ | | Packages containing at least one elevation read | **20** | ✅ | | Files containing at least one elevation read | 45 | ✅ | diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index 24a8099b15..da4d6b7663 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -94,6 +94,29 @@ export interface McpDataBridge { orderBy?: Array<{ field: string; order: 'asc' | 'desc' }>; }, ): Promise; + /** + * [ADR-0090 D10 — maintainer ruling 2026-09-08, #16549] Is a read on `object` + * NARROWED by the agent ceiling the caller runs under? + * + * The transport half of the ruling's consequence (2), and it exists because + * the deceived consumer on this surface is the **AI itself**: a delegated + * `query_records` answering `total: 0` with no note is read by the agent as a + * fact about the data, and it then tells a decision-maker "there are no + * opportunities this quarter". A narrowed count must arrive WITH the + * narrowing stated. + * + * OPTIONAL, with the same graceful-degradation contract as + * {@link McpDataBridge.listObjectsDiagnosed} and + * {@link McpDataBridge.aggregate}: a bridge bound to a principal that cannot + * be delegated (the stdio API-key host), or wired to a security service + * predating the probe, omits this member and the tool renders exactly what it + * rendered before. + * + * ⛔ `{ narrowed: false }` and "cannot say" are deliberately the same RENDERED + * outcome — neither may manufacture a warning. The tool states a narrowing + * only where one was established. + */ + diagnoseDelegation?(object: string): Promise<{ narrowed: boolean; statement?: string } | undefined>; get(object: string, id: string): Promise; create(object: string, data: Record): Promise; update(object: string, id: string, data: Record): Promise; @@ -231,6 +254,26 @@ function errorResult(message: string) { return { content: [{ type: 'text' as const, text: message }], isError: true as const }; } +/** + * [ADR-0090 D10 — ruling 2026-09-08, consequence 2] Attach the D10 narrowing + * notice to a query result WITHOUT reshaping it. + * + * The bridge's `query` is typed `Promise`, and the shape that ships is + * the protocol's `{ object, records, total }` — so the object case adds two + * keys beside the existing ones, mirroring `list_objects`' `partial` / `warning` + * pair so a client branches on the same vocabulary on both tools. Anything that + * is NOT a plain object (a bare array from some future bridge) keeps its own + * shape and carries the notice alongside: inventing a `records` wrapper there + * would break a caller to deliver a warning about breaking callers. + */ +function withDelegationNotice(value: unknown, statement: string): unknown { + const notice = { delegationNarrowed: true as const, warning: statement }; + if (value !== null && typeof value === 'object' && !Array.isArray(value)) { + return { ...(value as Record), ...notice }; + } + return { result: value, ...notice }; +} + function jsonText(value: unknown): string { try { return JSON.stringify(value, null, 2); @@ -494,7 +537,19 @@ export function registerObjectTools( offset, orderBy, }); - return textResult(result); + // [ADR-0090 D10 — ruling 2026-09-08, consequence 2] A delegated read + // the agent ceiling NARROWED is served with the narrowing stated. The + // rows are still served — a partial answer is the most useful true + // thing here, exactly as in `list_objects` above; what is withheld is + // the implicit claim that this count describes the object. + // + // ⛔ The probe may never fail the query it annotates: it is caught + // into "no statement", which is byte-for-byte the pre-#16549 render. + const diagnosed = typeof bridge.diagnoseDelegation === 'function' + ? await bridge.diagnoseDelegation(objectName).catch(() => undefined) + : undefined; + if (!diagnosed?.narrowed || !diagnosed.statement) return textResult(result); + return textResult(withDelegationNotice(result, diagnosed.statement)); } catch (err) { return errorResult(messageOf(err)); } diff --git a/packages/mcp/src/query-records-delegation-diagnostic.test.ts b/packages/mcp/src/query-records-delegation-diagnostic.test.ts new file mode 100644 index 0000000000..55262a36f4 --- /dev/null +++ b/packages/mcp/src/query-records-delegation-diagnostic.test.ts @@ -0,0 +1,115 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0090 D10 — maintainer ruling 2026-09-08, #16549 consequence 2] + * A delegated `query_records` NARROWED by the agent ceiling says so; an + * un-narrowed one does not. + * + * This is the half of the ruling that is INDEPENDENT of the parity half, and + * the half that matters most, because the deceived consumer on this surface is + * the AI itself. Measured on the card: `query_records` answered `total: 0` with + * no note, and the agent faithfully told a decision-maker "there are no + * opportunities this quarter" — a wrong answer delivered with full confidence, + * from a tool that was working exactly as specified. + * + * The rows are still served. What the notice adds is the one fact the payload + * could not previously carry: this count describes the CEILING, not the object. + * The shape mirrors `list_objects`' `partial` / `warning` pair (#6504) so a + * client branches on the same vocabulary across both tools. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; + +import { MCPServerRuntime } from './mcp-server-runtime.js'; +import type { McpDataBridge } from './mcp-http-tools.js'; + +const NARROWING = 'This result was narrowed by the ADR-0090 D10 intersection: rows are ABSENT from this result.'; + +function makeBridge( + diagnose?: McpDataBridge['diagnoseDelegation'], +): McpDataBridge & { calls: string[] } { + const calls: string[] = []; + const bridge: any = { + calls, + async listObjects() { return [{ name: 'crm_opportunity', label: 'Opportunity' }]; }, + async describeObject(name: string) { return { name }; }, + async query(object: string) { + calls.push('query'); + return { object, records: [], total: 0 }; + }, + async get(object: string, id: string) { return { object, id }; }, + async create(object: string, data: any) { return { object, data }; }, + async update(object: string, id: string) { return { object, id }; }, + async remove(object: string, id: string) { return { object, id, success: true }; }, + }; + if (diagnose) bridge.diagnoseDelegation = diagnose; + return bridge; +} + +async function queryRecords(runtime: MCPServerRuntime, bridge: any): Promise { + const body = { + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'query_records', arguments: { objectName: 'crm_opportunity' } }, + }; + const req = new Request('http://localhost/api/v1/mcp', { + method: 'POST', + headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' }, + body: JSON.stringify(body), + }); + const res = await runtime.handleHttpRequest(req, { bridge, parsedBody: body }); + const json: any = await res.json(); + expect(json.result?.isError).toBeFalsy(); + return JSON.parse(json.result.content[0].text); +} + +describe('query_records — ADR-0090 D10 delegated-read diagnostic (#16549)', () => { + let runtime: MCPServerRuntime; + + beforeEach(() => { + runtime = new MCPServerRuntime({ name: 't', version: '1.0.0' }); + }); + + it('a NARROWED delegated read carries the D10 statement beside the rows', async () => { + const bridge = makeBridge(async () => ({ narrowed: true, statement: NARROWING })); + const payload = await queryRecords(runtime, bridge); + // The rows are still served — a partial answer is the most useful true + // thing here. What is added is the withheld claim. + expect(payload.object).toBe('crm_opportunity'); + expect(payload.records).toEqual([]); + expect(payload.total).toBe(0); + expect(payload.delegationNarrowed).toBe(true); + expect(payload.warning).toBe(NARROWING); + }); + + it('an UN-NARROWED delegated read carries NO statement — the payload is byte-identical to before', async () => { + const bridge = makeBridge(async () => ({ narrowed: false })); + const payload = await queryRecords(runtime, bridge); + expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 }); + expect(payload.delegationNarrowed).toBeUndefined(); + expect(payload.warning).toBeUndefined(); + }); + + it('a bridge that cannot answer (no member at all) renders exactly what it rendered before', async () => { + // The stdio API-key host and any deployment whose security service predates + // the probe. ⛔ Absence must never be rendered as a warning — "cannot say" + // and "nothing to say" are deliberately the same rendered outcome. + const bridge = makeBridge(undefined); + const payload = await queryRecords(runtime, bridge); + expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 }); + }); + + it('a THROWING probe never fails the query it annotates', async () => { + const bridge = makeBridge(async () => { throw new Error('security service exploded'); }); + const payload = await queryRecords(runtime, bridge); + expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 }); + expect(bridge.calls).toContain('query'); + }); + + it('narrowed:true with NO statement adds nothing — a notice with no sentence is not a notice', async () => { + const bridge = makeBridge(async () => ({ narrowed: true })); + const payload = await queryRecords(runtime, bridge); + expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 }); + }); +}); diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index aa05bdae78..75a44509fd 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -663,6 +663,86 @@ export function narrowerScope(a: string, b: string): string { return rank(a) <= rank(b) ? a : b; } +/** + * [ADR-0090 D10 — maintainer ruling 2026-09-08, option 1] The depth the + * DELEGATED principal's own leg contributes, and whether that leg is what + * narrowed the read. + * + * **The rule: a ceiling that says nothing subtracts nothing.** `ceilingScope` + * is `PermissionEvaluator.getDeclaredScope` — `undefined` when every set the + * delegated principal holds is silent about depth. Silence is "no opinion", so + * this leg contributes NO owner narrowing (`'org'`) and every row bound comes + * from the delegator's leg, which plugin-sharing AND-s in beside it. A ceiling + * that DOES declare a depth contributes exactly that depth, unchanged. + * + * ⚠️ This is deliberately NOT `narrowerScope(ceiling, delegator)`. The + * OWD/sharing owner-match is IDENTITY-scoped: `__readScope` bounds the AGENT + * identity's owner-match and `__delegatorReadScope` bounds the DELEGATOR's, and + * the true intersection is the AND of the two filters (`sharing-plugin.ts`). + * Folding the minimum into this leg would scope the agent identity to the + * delegator's depth — `owner_id = agentId` at the delegator's depth — hiding + * the very rows the delegator legitimately owns. + * + * Why the rule exists, in one measurement: `mcp_agent_data_read` / + * `mcp_agent_data_write` are pure CAPABILITY ceilings — `{'*': {allowRead: + * true, …}}`, no `readScope`, no `viewAllRecords`, and their own doc comment + * says "NO row-level security … all row/owner/tenant narrowing comes from the + * delegating user". `getEffectiveScope` nonetheless answered `'own'` for them + * (its owner-only default for a granting-but-silent set), so this leg imposed + * an owner-match nobody declared and a `viewAllRecords` sales manager who reads + * 9 accounts / 23 opportunities / 45 tasks through the Console or a per-user + * API key read 5 / 0 / 0 through an OAuth MCP client — silently, `total: 0`, + * with the agent then reporting "there are no opportunities this quarter" as + * fact. + * + * ⛔ This widens VISIBILITY DEPTH only, and only on the SILENT-ceiling input. + * Every other subtraction is decided elsewhere and is unchanged — see the + * subtraction table on `MCP_AGENT_PERMISSION_SET_READ` in + * `objects/default-permission-sets.ts`. + * + * `narrowedByCeiling` is the diagnostic half of the same ruling: true iff the + * ceiling's OWN declared depth is strictly narrower than the delegator's, i.e. + * the delegated read is answering from a row set THIS leg shrank — and the MCP + * `query_records` result must then SAY so rather than serve a bare count. + */ +export function intersectDelegatedScope( + ceilingScope: string | undefined, + delegatorScope: string, +): { agentLegScope: string; narrowedByCeiling: boolean } { + if (ceilingScope === undefined) { + return { agentLegScope: 'org', narrowedByCeiling: false }; + } + return { + agentLegScope: ceilingScope, + narrowedByCeiling: narrowerScope(ceilingScope, delegatorScope) !== delegatorScope, + }; +} + +/** + * [ADR-0090 D10 — ruling 2026-09-08, consequence 2] The sentence a delegated + * read carries when the ceiling narrowed it. + * + * ONE spelling, so the explain path (`layer: 'object_crud'` / `depth`) and the + * MCP tool result cannot describe the same intersection differently. It states + * the DIRECTION of the error — the row set is a subset, the count is not a fact + * about the data — because the consumer being corrected is an AI that would + * otherwise report `total: 0` as an answer. + */ +export function d10NarrowingStatement(input: { + object: string; + delegatorId: string; + effectiveScope: string; + delegatorScope: string; +}): string { + return ( + `This result was narrowed by the ADR-0090 D10 intersection: the agent principal acting on behalf of ` + + `${input.delegatorId} is capped at '${input.effectiveScope}' record depth on ${input.object}, while ` + + `${input.delegatorId} alone reaches '${input.delegatorScope}'. Rows outside that depth are ABSENT from ` + + `this result and are NOT absent from the object — do not report this count as a fact about the data. ` + + `Ask ${input.delegatorId} to re-run the question under their own credentials to see the full set.` + ); +} + /** * [ADR-0090 D10] Intersect two FLS masks. A field is readable/editable in the * result only if it is readable/editable on BOTH sides. A field ABSENT from a @@ -1280,10 +1360,24 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput // ── 6. depth ─────────────────────────────────────────────────────────── const opClass = dataOp === 'find' ? 'read' : 'write'; const agentScope = deps.evaluator.getEffectiveScope(opClass as 'read' | 'write', object, sets, { isPrivate: secMeta.isPrivate }); - // [ADR-0090 D10] The delegated principal sees the NARROWER of the two depths. - const scope = delegatorSets - ? narrowerScope(agentScope, deps.evaluator.getEffectiveScope(opClass as 'read' | 'write', object, delegatorSets, { isPrivate: secMeta.isPrivate })) - : agentScope; + // [ADR-0090 D10 — ruling 2026-09-08] The delegated row set is the AND of the + // two identity legs, so the depth a reader of this report cares about is the + // narrower of them — and a ceiling that DECLARES no depth contributes no + // narrowing at all (`intersectDelegatedScope`). The agent leg here is the + // SAME value the middleware stashes as `__readScope` (`security-plugin.ts` + // step 2.6), computed by the SAME function from the SAME two evaluator calls, + // so a report saying "narrowed to 'own'" can never sit beside a query that + // was not narrowed. + const delegatorScope = delegatorSets + ? deps.evaluator.getEffectiveScope(opClass as 'read' | 'write', object, delegatorSets, { isPrivate: secMeta.isPrivate }) + : null; + const delegated = delegatorSets + ? intersectDelegatedScope( + deps.evaluator.getDeclaredScope(opClass as 'read' | 'write', object, sets, { isPrivate: secMeta.isPrivate }), + delegatorScope!, + ) + : null; + const scope = delegated ? narrowerScope(delegated.agentLegScope, delegatorScope!) : agentScope; const depthApplies = owd.effect !== 'public'; layers.push({ layer: 'depth', @@ -1291,7 +1385,11 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput detail: !depthApplies ? 'Depth axis does not apply (baseline already org-wide).' : `Effective ${opClass} depth: '${scope}' (ADR-0057 D1 — widest across granting sets; ` + - (delegatorSets ? `narrowed to the delegator's depth by D10 intersection; ` : '') + + (delegated + ? delegated.narrowedByCeiling + ? `narrowed from the delegator's '${delegatorScope}' to the agent ceiling's '${delegated.agentLegScope}' by the D10 intersection; ` + : `the agent ceiling declares no depth, so the delegator's '${delegatorScope}' stands under the D10 intersection; ` + : '') + `assignment BU anchors narrow which unit 'unit*' means, ADR-0090 Addendum).`, contributors: [], }); diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts index 3b91f54b29..f62e5e9ea8 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts @@ -1057,6 +1057,57 @@ const baseDefaultPermissionSets: PermissionSet[] = [ // to a position or an audience anchor — the producer // (`resolve-execution-context`) injects them onto the agent principal's // context directly — so the anchor high-privilege gate does not apply. + // + // ── WHAT EACH SCOPE STILL SUBTRACTS ───────────────────────────────────── + // [maintainer ruling 2026-09-08, issue #16549, option 1] The ruling widened + // exactly ONE axis — record-visibility DEPTH — and required the remaining + // subtractions to be written down, because "a subtraction nobody wrote down + // is the next card". These sets declare no `readScope` / `writeScope` and no + // `viewAllRecords` / `modifyAllRecords`, which the D10 fold now reads as + // NO OPINION on depth (`PermissionEvaluator.getDeclaredScope` → + // `intersectDelegatedScope`): the delegator's own depth stands, so + // `agent ∩ user = user` for visibility and the Setup page's promise — + // "every call runs under the caller's own permissions and row-level + // security" — is true rather than rewritten. ⛔ Everything below is what the + // ceiling still removes, and none of it moved: + // + // `data:read` → `mcp_agent_data_read` + // · NO write of any kind. `allowCreate` / `allowEdit` / `allowDelete` are + // absent, so the CRUD gate refuses insert/update/delete for ANY + // delegator, including a platform admin. (The tool surface is narrowed + // too — `registerObjectTools` does not register the write tools without + // `data:write` — but the DATA-LAYER refusal here is the enforced one.) + // · NO `allowTransfer`: an ownership transfer is refused even where the + // delegator holds it (`security-plugin.ts` transfer gate — BOTH sides + // must grant it). + // · NO `allowExport`. + // · NO `systemPermissions`, so every capability-gated object stays denied + // regardless of the delegator's capabilities. + // · `private`-posture objects are NOT covered: a `'*'` wildcard without a + // superuser bit does not reach them (`resolveObjectPermission`), so the + // agent is denied outright — a loud refusal, never a narrowed count. + // + // `data:write` → `mcp_agent_data_write` + // · Everything above except the write bits, PLUS: + // · `sys_*` / better-auth-managed identity tables stay READ-ONLY via + // `denyWritesOnManagedObjects()` — the one arm that survives even an + // admin delegator. + // · NO `allowTransfer` (unchanged: the write ceiling never carried it). + // · NO `allowExport`. + // + // neither scope → `mcp_agent_restricted` + // · `objects: {}` — no object reaches the agent at all; the resolved list + // is non-empty only so enforcement fails CLOSED. + // + // ALL scopes, on every path + // · The delegator's own grants still bound everything: CRUD, FLS masks, + // Layer 0 tenant wall, Layer 1 RLS and record sharing are each AND-ed + // across both principals (`security-plugin.ts`, ADR-0090 D10). + // · A dangling delegator (`sys_user` gone) fails CLOSED. + // · Share-MANAGEMENT authority is not delegated: `ISecurityService`'s + // `hasWriteBypass` → `false` and `resolveWriteScope` → `'own'` for any + // on-behalf-of context, so `ISharingService.canManageShares` refuses. + // ⛔ Deliberately left in place by the ruling, which names visibility. PermissionSetSchema.parse({ name: MCP_AGENT_PERMISSION_SET_READ, label: 'MCP Agent — Read Only', diff --git a/packages/plugins/plugin-security/src/permission-evaluator.ts b/packages/plugins/plugin-security/src/permission-evaluator.ts index 70e3b5f73e..d8ced6c6f8 100644 --- a/packages/plugins/plugin-security/src/permission-evaluator.ts +++ b/packages/plugins/plugin-security/src/permission-evaluator.ts @@ -271,6 +271,47 @@ export class PermissionEvaluator { return ORDER[widest < 0 ? 0 : widest]; } + /** + * [ADR-0090 D10 — ruling 2026-09-08] The access DEPTH these sets actually + * **declare** for an operation class, or `undefined` when they declare none. + * + * The same walk as {@link getEffectiveScope}, minus its two DEFAULTS. Those + * defaults are right for a principal standing on its own — a granting set + * silent about depth is owner-only, and an object no set mentions is denied + * separately — but they are *manufactured* opinions, and the ADR-0090 D10 + * intersection must not subtract with one. `undefined` = "no opinion": every + * set that grants the op is silent on depth (no `readScope` / `writeScope`, + * no `viewAllRecords` / `modifyAllRecords`), or no set mentions the object at + * all. A set that DOES declare a depth answers here exactly as + * `getEffectiveScope` would — widest wins — so a declared ceiling keeps its + * full subtractive force. + * + * ⛔ Not a replacement for `getEffectiveScope`: the non-delegated path still + * needs the owner-only default. This is the DELEGATED path's input, folded by + * `intersectDelegatedScope` (explain-engine.ts). + */ + getDeclaredScope( + opClass: 'read' | 'write', + objectName: string, + permissionSets: PermissionSet[], + opts: { isPrivate?: boolean } = {}, + ): 'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined { + const RANK = { own: 0, own_and_reports: 1, unit: 2, unit_and_below: 3, org: 4 } as const; + const ORDER = ['own', 'own_and_reports', 'unit', 'unit_and_below', 'org'] as const; + let widest = -1; + for (const ps of permissionSets) { + const op: any = resolveObjectPermission(ps, objectName, opts.isPrivate ?? false); + if (!op) continue; + if (opClass === 'read' && (op.viewAllRecords || op.modifyAllRecords)) return 'org'; + if (opClass === 'write' && op.modifyAllRecords) return 'org'; + const s = opClass === 'read' ? op.readScope : op.writeScope; + if (!s) continue; + const rank = RANK[s as keyof typeof RANK]; + if (rank != null && rank > widest) widest = rank; + } + return widest < 0 ? undefined : ORDER[widest]; + } + /** * [ADR-0066 D3] Union of `systemPermissions` (capabilities) the caller holds * across the resolved permission sets — used to enforce a resource's diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index eb688e2ad3..562ffc4ea9 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -3848,9 +3848,16 @@ describe('SecurityPlugin — ADR-0090 D10 agent intersection', () => { objectql: ql, metadata: { get: async () => baseSchema, list: async () => opts.sets }, }; + // The registered `security` service is captured, not discarded: the + // ADR-0090 D10 diagnostic (`describeDelegationNarrowing`) is published ON + // that service, and asserting it through the same boot the enforcement + // tests use is what keeps the two from drifting. + let registeredSecurity: any = null; const ctx: any = { logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - registerService: vi.fn(), + registerService: vi.fn((name: string, impl: any) => { + if (name === 'security') registeredSecurity = impl; + }), getService: (name: string) => { if (!(name in services)) throw new Error(`service not registered: ${name}`); return services[name]; @@ -3858,6 +3865,7 @@ describe('SecurityPlugin — ADR-0090 D10 agent intersection', () => { }; return { ctx, taskFindOne, + security: () => registeredSecurity, run: async (opCtx: any) => { await middleware(opCtx, () => runEngineWriteBody(opCtx)); return opCtx; }, }; }; @@ -3984,6 +3992,193 @@ describe('SecurityPlugin — ADR-0090 D10 agent intersection', () => { expect(ctx.__delegatorReadScope).toBe('own'); }); + // ── [#16549 / maintainer ruling 2026-09-08] "a ceiling that says nothing + // subtracts nothing" — the OAuth-agent parity half of the ruling ───────── + // + // The shipped MCP ceilings (`mcp_agent_data_read` / `_write`) are pure + // CAPABILITY sets: a `'*'` grant with no `readScope` and no `viewAllRecords`. + // `getEffectiveScope`'s owner-only default turned that silence into `'own'`, + // so the agent leg imposed an owner-match nobody declared and every + // `viewAllRecords` manager collapsed to `own + shares` the moment an OAuth + // MCP client asked on her behalf: measured 9/23/45 through an API key, + // 5/0/0 over OAuth, same account, same questions, same server. + // + // `silentCeiling` is that exact shape. The pins below fix BOTH directions — + // what the ruling widened, and every subtraction it did not. + const silentCeiling = (name: string, extra?: Record): PermissionSet => ({ + name, label: name, objects: { task: { allowRead: true, ...(extra ?? {}) } }, + } as any); + const viewAllDelegator = (name: string): PermissionSet => ({ + name, label: name, objects: { task: { allowRead: true, viewAllRecords: true } }, + } as any); + + it('[#16549] a ceiling silent about depth contributes NO owner narrowing — the delegator\'s viewAllRecords stands', async () => { + const { h } = await boot({ + sets: [silentCeiling('agent_set'), viewAllDelegator('del_set')], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + schemaExtra: { sharingModel: 'private' }, + }); + const ctx: any = agentCtx(); + await h.run({ object: 'task', operation: 'find', ast: { where: undefined }, context: ctx }); + // Was 'own' before the ruling — the manufactured opinion that produced + // `crm_opportunity: 0`. Both legs now say 'org', so plugin-sharing's + // `buildReadFilter` returns null on each and the agent reads exactly the + // rows the human reads. THIS is pin 1: equal, not merely closer. + expect(ctx.__readScope).toBe('org'); + expect(ctx.__delegatorReadScope).toBe('org'); + }); + + it('[#16549] NEGATIVE CONTROL: a delegator WITHOUT viewAllRecords is unchanged — the delegator leg still bounds the read', async () => { + const delOwn: PermissionSet = { name: 'del_set', label: 'd', objects: { task: { allowRead: true, readScope: 'own' } } } as any; + const { h } = await boot({ + sets: [silentCeiling('agent_set'), delOwn], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + schemaExtra: { sharingModel: 'private' }, + }); + const ctx: any = agentCtx(); + await h.run({ object: 'task', operation: 'find', ast: { where: undefined }, context: ctx }); + // The widening is on the CEILING's leg only. The delegator's own 'own' + // depth is untouched and still AND-s in, so a user with no viewAllRecords + // sees exactly what she saw before: own + shares. ⛔ The fix widens the + // manager's view, not everyone's. + expect(ctx.__readScope).toBe('org'); + expect(ctx.__delegatorReadScope).toBe('own'); + }); + + it('[#16549] NEGATIVE CONTROL: a ceiling that DOES declare a depth keeps its full subtractive force', async () => { + const declaringCeiling: PermissionSet = { name: 'agent_set', label: 'a', objects: { task: { allowRead: true, readScope: 'own' } } } as any; + const { h } = await boot({ + sets: [declaringCeiling, viewAllDelegator('del_set')], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + schemaExtra: { sharingModel: 'private' }, + }); + const ctx: any = agentCtx(); + await h.run({ object: 'task', operation: 'find', ast: { where: undefined }, context: ctx }); + // Silence is "no opinion"; a DECLARATION is an opinion and still narrows. + expect(ctx.__readScope).toBe('own'); + expect(ctx.__delegatorReadScope).toBe('org'); + }); + + it('[#16549] the write leg follows the same rule — a silent write ceiling takes the delegator\'s modifyAllRecords depth', async () => { + const silentWriteCeiling: PermissionSet = { + name: 'agent_set', label: 'a', + objects: { task: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + } as any; + const modifyAllDelegator: PermissionSet = { + name: 'del_set', label: 'd', + objects: { task: { allowRead: true, allowEdit: true, allowDelete: true, modifyAllRecords: true } }, + } as any; + const { h } = await boot({ + sets: [silentWriteCeiling, modifyAllDelegator], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + schemaExtra: { sharingModel: 'private' }, + }); + const ctx: any = agentCtx(); + await h.run({ object: 'task', operation: 'update', data: { id: 'r1', name: 'x' }, options: { where: { id: 'r1' } }, context: ctx }); + expect(ctx.__writeScope).toBe('org'); + expect(ctx.__delegatorWriteScope).toBe('org'); + }); + + it('[#16549] NEGATIVE CONTROL: the read-only ceiling still cannot WRITE, whatever the delegator holds', async () => { + // The shipped `mcp_agent_data_read` shape against a delegator who CAN edit + // and holds modifyAllRecords. Depth is now 'org' on the agent leg — and the + // CRUD gate refuses anyway, because depth and capability are different axes + // and the ruling moved only the first. + const modifyAllDelegator: PermissionSet = { + name: 'del_set', label: 'd', + objects: { task: { allowRead: true, allowEdit: true, modifyAllRecords: true } }, + } as any; + const { h } = await boot({ + sets: [silentCeiling('agent_set'), modifyAllDelegator], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + }); + await expect(h.run({ + object: 'task', operation: 'update', data: { id: 'r1', name: 'x' }, + options: { where: { id: 'r1' } }, context: agentCtx(), + })).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('[#16549] NEGATIVE CONTROL: allowTransfer is still refused — a write ceiling without it cannot move owner_id', async () => { + const silentWriteCeiling: PermissionSet = { + name: 'agent_set', label: 'a', + objects: { task: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + } as any; + const transferringDelegator: PermissionSet = { + name: 'del_set', label: 'd', + objects: { task: { allowRead: true, allowEdit: true, modifyAllRecords: true } }, + } as any; + const { h } = await boot({ + sets: [silentWriteCeiling, transferringDelegator], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + }); + // The delegator holds transfer via modifyAllRecords; the ceiling does not + // declare `allowTransfer` and D10 requires BOTH. ⛔ The ruling explicitly + // fenced this off — and it is the reason the fix lands on the intersection + // rather than putting `modifyAllRecords` on the ceiling, which would have + // granted transfer (MODIFY_ALL_WRITE_KEYS covers it) and reached `private` + // objects through the superuser wildcard. + await expect(h.run({ + object: 'task', operation: 'update', data: { id: 'r1', owner_id: 'someone_else' }, + options: { where: { id: 'r1' } }, context: agentCtx(), + })).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + // ── [#16549 consequence 2] the diagnostic, INDEPENDENT of the parity half ── + describe('describeDelegationNarrowing — a narrowed delegated read says so', () => { + it('reports the narrowing when the ceiling declares a depth below the delegator\'s', async () => { + const declaringCeiling: PermissionSet = { name: 'agent_set', label: 'a', objects: { task: { allowRead: true, readScope: 'own' } } } as any; + const { h } = await boot({ + sets: [declaringCeiling, viewAllDelegator('del_set')], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + }); + const verdict = await h.security().describeDelegationNarrowing('task', agentCtx()); + expect(verdict.narrowed).toBe(true); + expect(verdict.effectiveScope).toBe('own'); + expect(verdict.delegatorScope).toBe('org'); + // The sentence is for an AI consumer: it must say the rows are ABSENT + // from the result rather than from the object, or the agent reports + // `total: 0` as an answer — the whole cost of this card. + expect(verdict.statement).toMatch(/D10 intersection/); + expect(verdict.statement).toMatch(/NOT absent from the object/); + expect(verdict.statement).toContain(DELEGATOR); + expect(verdict.statement).toContain('task'); + }); + + it('reports NO narrowing for a silent ceiling — an un-narrowed delegated read carries no statement', async () => { + const { h } = await boot({ + sets: [silentCeiling('agent_set'), viewAllDelegator('del_set')], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + }); + const verdict = await h.security().describeDelegationNarrowing('task', agentCtx()); + expect(verdict).toEqual({ narrowed: false }); + }); + + it('reports NO narrowing for a NON-delegated principal', async () => { + const { h } = await boot({ + sets: [silentCeiling('agent_set'), viewAllDelegator('del_set')], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + }); + const humanCtx = { userId: DELEGATOR, tenantId: 'org-1', positions: ['del_set'], permissions: [] }; + expect(await h.security().describeDelegationNarrowing('task', humanCtx)).toEqual({ narrowed: false }); + }); + + it('reports NO narrowing for a system context — a diagnostic never speaks for the engine\'s own writer', async () => { + const { h } = await boot({ + sets: [silentCeiling('agent_set'), viewAllDelegator('del_set')], + agentPositions: ['agent_set'], delegatorPositions: ['del_set'], + }); + expect(await h.security().describeDelegationNarrowing('task', { isSystem: true })).toEqual({ narrowed: false }); + }); + + it('a DANGLING delegator is a denial upstream, not a narrowing — the probe stays silent', async () => { + const { h } = await boot({ + sets: [silentCeiling('agent_set')], + agentPositions: ['agent_set'], delegatorPositions: null, + }); + expect(await h.security().describeDelegationNarrowing('task', agentCtx())).toEqual({ narrowed: false }); + }); + }); + // ── fail-closed on a dangling delegation link ─────────────────────────── it('delegator no longer exists → fail CLOSED (PermissionDeniedError, not baseline access)', async () => { const { h } = await boot({ sets: [reader('agent_set')], agentPositions: ['agent_set'], delegatorPositions: null }); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index b6dc708a7c..57b8a8e2c2 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -28,6 +28,8 @@ import { buildContextForUser, resolveDelegatorContext, intersectFieldMasks, + intersectDelegatedScope, + d10NarrowingStatement, } from './explain-engine.js'; import type { ExplainDecision, ExplainOperation } from '@objectstack/spec/security'; import type { II18nService, IMetadataService, IObjectQLEngine } from '@objectstack/spec/contracts'; @@ -94,6 +96,7 @@ import { normalizeTenancyPosture, postureEnforcesWall, postureUsesUnionScope, + type ObjectAccessScope, type TenancyPosture, } from '@objectstack/spec/security'; import { PLATFORM_CAPABILITY_NAMES } from '@objectstack/spec/security'; @@ -105,6 +108,7 @@ import { type SharingWriteVerdict, type AuthoredRowWriteVerdict, type AuthoredRowWriteOperation, + type DelegationNarrowing, } from '@objectstack/spec/contracts'; import { matchesFilterCondition } from '@objectstack/formula'; import { FieldMasker } from './field-masker.js'; @@ -1573,6 +1577,58 @@ export class SecurityPlugin implements Plugin { return 'own'; } }, + // [ADR-0090 D10 — maintainer ruling 2026-09-08, consequence 2] The + // delegated-read diagnostic. NOT an enforcement path: it decides + // nothing, narrows nothing, and never throws outward. It answers the + // one question a transport serving a delegated read owes its caller — + // "is this count a fact about the object, or about the ceiling?" — + // because the caller is an AI that will otherwise report `total: 0` as + // an answer (the measured failure on #16549). + // + // Resolved from the SAME two evaluator calls the CRUD middleware makes + // when it stashes `__readScope` (step 2.6), folded by the SAME + // `intersectDelegatedScope`. That is what stops it becoming a phantom + // check: it cannot report a narrowing the query did not have, and it + // cannot miss one the query did. + describeDelegationNarrowing: async (object: string, context?: any): Promise => { + const none: DelegationNarrowing = { narrowed: false }; + if (context?.isSystem) return none; + const delegatorId = context?.onBehalfOf?.userId; + if (!context?.userId || !delegatorId) return none; + try { + const del = await resolveDelegatorContext(this.ql, context); + // A dangling delegator is a fail-CLOSED DENIAL upstream, not a + // narrowing — the read never returns rows for this to describe. + if (del.kind !== 'resolved') return none; + const meta = await this.getObjectSecurityMeta(object); + const sets = await this.resolvePermissionSetsForContext(context); + const delegatorSets = await this.resolvePermissionSetsForContext(del.context); + const delegatorScope = this.permissionEvaluator.getEffectiveScope('read', object, delegatorSets, { isPrivate: meta.isPrivate }); + const declared = this.permissionEvaluator.getDeclaredScope('read', object, sets, { isPrivate: meta.isPrivate }); + const folded = intersectDelegatedScope(declared, delegatorScope); + if (!folded.narrowedByCeiling) return none; + return { + narrowed: true, + statement: d10NarrowingStatement({ + object, + delegatorId: String(delegatorId), + effectiveScope: folded.agentLegScope, + delegatorScope, + }), + effectiveScope: folded.agentLegScope as ObjectAccessScope, + delegatorScope, + }; + } catch (e) { + // ⛔ A diagnostic must never fail a read. Silence is the only safe + // direction: it degrades to exactly the behaviour that shipped + // before this method existed. + this.logger.warn?.( + `[security] describeDelegationNarrowing failed for object '${object}' — reporting no narrowing`, + e instanceof Error ? e : new Error(String(e)), + ); + return none; + } + }, // [#5493 / ADR-0105 D3] Authored-row-write evidence: does an // APP-AUTHORED (non-floor) RLS policy admit this row for this write, // with the platform's `created_by` ownership floor taken out by @@ -2247,10 +2303,31 @@ export class SecurityPlugin implements Plugin { // (plugin-sharing), so we pass the scope STRING, not the resolved set. if (permissionSets.length > 0) { const sc: any = opCtx.context; + // [ADR-0090 D10 — maintainer ruling 2026-09-08, option 1] The DELEGATED + // principal's own depth is its ceiling's DECLARED depth, and a ceiling + // that declares none says nothing about visibility — so the delegator's + // depth stands (`intersectDelegatedScope`). Before this, the ceiling's + // silence was read as `own` by `getEffectiveScope`'s owner-only default + // and every `viewAllRecords` manager collapsed to `own + shares` the + // moment an OAuth MCP client asked on their behalf. + // + // ⛔ VISIBILITY DEPTH only. The ceiling's CRUD bits (step 2.5 above), + // its `allowTransfer` refusal (step 2.9), its managed-object write + // denies and its private-object exclusion are all decided elsewhere and + // are untouched — see the subtraction table on the ceiling sets. + const depthFor = (opClass: 'read' | 'write'): { agentLegScope: string; delegatorScope: string } | null => { + if (!delegatorSets) return null; + const delegatorScope = this.permissionEvaluator.getEffectiveScope(opClass, opCtx.object, delegatorSets, { isPrivate: secMeta.isPrivate }); + const declared = this.permissionEvaluator.getDeclaredScope(opClass, opCtx.object, permissionSets, { isPrivate: secMeta.isPrivate }); + return { agentLegScope: intersectDelegatedScope(declared, delegatorScope).agentLegScope, delegatorScope }; + }; // The AGENT's own depth drives plugin-sharing's owner-match for the // agent identity (unchanged on the non-delegated path). if (['find', 'findOne', 'count', 'aggregate'].includes(opCtx.operation)) { - sc.__readScope = this.permissionEvaluator.getEffectiveScope('read', opCtx.object, permissionSets, { isPrivate: secMeta.isPrivate }); + const delegated = depthFor('read'); + sc.__readScope = delegated + ? delegated.agentLegScope + : this.permissionEvaluator.getEffectiveScope('read', opCtx.object, permissionSets, { isPrivate: secMeta.isPrivate }); // [ADR-0090 D10] Stash the DELEGATOR's own read depth SEPARATELY (not a // min of the two). The OWD/sharing owner-match is identity-scoped: // plugin-sharing re-runs the owner filter under the delegator's @@ -2258,14 +2335,13 @@ export class SecurityPlugin implements Plugin { // intersection. Narrowing __readScope alone would wrongly scope the // AGENT's identity to the delegator's depth (owner_id = agentId), // hiding the very rows the delegator legitimately owns. - if (delegatorSets) { - sc.__delegatorReadScope = this.permissionEvaluator.getEffectiveScope('read', opCtx.object, delegatorSets, { isPrivate: secMeta.isPrivate }); - } + if (delegated) sc.__delegatorReadScope = delegated.delegatorScope; } else if (['update', 'delete', 'transfer', 'restore', 'purge'].includes(opCtx.operation)) { - sc.__writeScope = this.permissionEvaluator.getEffectiveScope('write', opCtx.object, permissionSets, { isPrivate: secMeta.isPrivate }); - if (delegatorSets) { - sc.__delegatorWriteScope = this.permissionEvaluator.getEffectiveScope('write', opCtx.object, delegatorSets, { isPrivate: secMeta.isPrivate }); - } + const delegated = depthFor('write'); + sc.__writeScope = delegated + ? delegated.agentLegScope + : this.permissionEvaluator.getEffectiveScope('write', opCtx.object, permissionSets, { isPrivate: secMeta.isPrivate }); + if (delegated) sc.__delegatorWriteScope = delegated.delegatorScope; } } diff --git a/packages/qa/dogfood/test/showcase-agent-scope-ceiling.dogfood.test.ts b/packages/qa/dogfood/test/showcase-agent-scope-ceiling.dogfood.test.ts index 65cf227e58..bad3a6a9b3 100644 --- a/packages/qa/dogfood/test/showcase-agent-scope-ceiling.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-agent-scope-ceiling.dogfood.test.ts @@ -30,6 +30,8 @@ describe('showcase: ADR-0090 D10 agent scope ceiling (served engine)', () => { let ql: any; let aliceTok: string; let aliceId: string; + let adminTok: string; + let adminId: string; let noteId: string; const uid = async (email: string) => @@ -37,10 +39,11 @@ describe('showcase: ADR-0090 D10 agent scope ceiling (served engine)', () => { beforeAll(async () => { stack = await getSharedShowcase(); - await stack.signIn(); // admin bootstrap + adminTok = await stack.signIn(); // admin bootstrap aliceTok = await stack.signUp('scope-alice@verify.test'); ql = await stack.kernel.getServiceAsync('objectql'); aliceId = await uid('scope-alice@verify.test'); + adminId = await uid('admin@objectos.ai'); // Alice (a plain member) owns a private note — she can read AND edit it. const created = await stack.apiAs(aliceTok, 'POST', '/data/showcase_private_note', { title: 'Alice note' }); @@ -94,4 +97,81 @@ describe('showcase: ADR-0090 D10 agent scope ceiling (served engine)', () => { const after = await ql.findOne('showcase_private_note', { where: { id: noteId }, context: SYS }); expect(after?.title).toBe('write agent edit'); }); + + // ───────────────────────────────────────────────────────────────────────── + // [#16549 / maintainer ruling 2026-09-08, decision batch #81 item 1] + // The measured table, end to end: a user whose visibility comes from + // `viewAllRecords` must read the SAME rows through an OAuth agent as through + // her own credentials. ⛔ Not "closer" — equal. + // + // Reported: same account, same questions, same server — + // API key, `principalKind: human` → 9 accounts / 23 opportunities / 45 tasks + // OAuth, `principalKind: agent` → 5 / 0 / 0 + // and the follow-up narrowed it to one column: re-owning every row to the + // demo user made OAuth answer 9/23/59, so the agent path was reading `own` + // where the human path read `viewAllRecords`, and the profile grant was not + // consulted for the agent at all. + // + // The dev admin is this stack's `viewAllRecords` profile (a `'*'` wildcard + // carrying the superuser bits) and owns none of Alice's rows, which is + // exactly the shape the card measured. The human leg goes over the REST door + // with a real token — the same door the API-key row was measured through — + // and the agent leg through the engine with the producer's own context, so + // the two legs really are two identity paths onto one row. + // ───────────────────────────────────────────────────────────────────────── + const agentForAdmin = (ceiling: string) => ({ + userId: adminId, + principalKind: 'agent' as const, + positions: [] as string[], + permissions: [ceiling], + onBehalfOf: { userId: adminId, principalKind: 'human' as const }, + }); + + const restIds = async (token: string): Promise => { + const res = await stack.apiAs(token, 'GET', '/data/showcase_private_note?$top=200'); + expect(res.status, 'REST read succeeds').toBeLessThan(300); + const body: any = await res.json(); + const rows: any[] = body?.data?.records ?? body?.records ?? body?.data ?? body?.value ?? []; + return (Array.isArray(rows) ? rows : []).map(idOf).filter(Boolean); + }; + + it("[#16549] PARITY: the human leg and the OAuth agent leg agree on Alice's note for a viewAllRecords profile", async () => { + // Human leg — the admin reads Alice's row because her profile carries + // `viewAllRecords`, not because she owns it. + const humanIds = await restIds(adminTok); + expect(humanIds, 'human leg sees the row it does not own').toContain(noteId); + + // Agent leg — same person, OAuth `data:read`. Before the ruling this leg + // ran at `own` and returned nothing: the admin owns no notes. + const agentRows = await ql.find('showcase_private_note', { where: {}, context: agentForAdmin('mcp_agent_data_read') }); + const agentIds = (agentRows ?? []).map(idOf); + expect(agentIds, 'agent leg sees the same row').toContain(noteId); + + // ⛔ EQUAL, not merely non-empty: every row the human leg reached, the + // agent leg reached too. Containment in this direction is the whole claim, + // and it is asserted over rows this suite did not create as well — a + // shared-stack-safe way of saying "the two rows of the table match". + expect(agentIds).toEqual(expect.arrayContaining(humanIds)); + }); + + it('[#16549] NEGATIVE CONTROL: an agent for a member WITHOUT viewAllRecords still sees only her own rows', async () => { + // The admin owns a note; Alice has no viewAllRecords and no share on it. + // Her agent must not gain sight of it — the fix widens the manager's view, + // not everyone's. + const created = await stack.apiAs(adminTok, 'POST', '/data/showcase_private_note', { title: 'Admin-owned note (D10 control)' }); + expect(created.status, 'admin creates a note she owns').toBeLessThan(300); + const adminNoteId = idOf(await created.json()) + ?? (await ql.findOne('showcase_private_note', { where: { title: 'Admin-owned note (D10 control)' }, context: SYS }))?.id; + expect(adminNoteId, 'admin note id resolved').toBeTruthy(); + + const aliceAgentIds = ((await ql.find('showcase_private_note', { where: {}, context: agentCtx('mcp_agent_data_read') })) ?? []).map(idOf); + expect(aliceAgentIds, "Alice's agent still reaches her own row").toContain(noteId); + expect(aliceAgentIds, "Alice's agent does NOT reach a row she does not own").not.toContain(adminNoteId); + + // …while the admin's own agent does — the same query, the same object, the + // only difference being whose profile is on the other side of the D10 + // intersection. That difference is what the ruling restored. + const adminAgentIds = ((await ql.find('showcase_private_note', { where: {}, context: agentForAdmin('mcp_agent_data_read') })) ?? []).map(idOf); + expect(adminAgentIds).toContain(adminNoteId); + }); }); diff --git a/packages/runtime/src/domains/mcp.ts b/packages/runtime/src/domains/mcp.ts index db15aef36e..50a7d92f69 100644 --- a/packages/runtime/src/domains/mcp.ts +++ b/packages/runtime/src/domains/mcp.ts @@ -11,6 +11,7 @@ import { isMcpServerEnabled } from '@objectstack/types'; import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; import type { MetadataProtocol } from '@objectstack/spec/api'; +import type { ISecurityService } from '@objectstack/spec/contracts'; import { buildApiError } from '../error-envelope.js'; import * as actionExec from '../action-execution.js'; import { isSystemObjectName } from '../action-execution.js'; @@ -570,6 +571,36 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon if (o?.orderBy) query.orderBy = o.orderBy; return await callData('query', { object, query }, driver, envId, ec); }, + /** + * [ADR-0090 D10 — maintainer ruling 2026-09-08, #16549] The + * delegated-read diagnostic, asked of THIS request's security service + * about THIS request's principal. + * + * `query_records` renders it only where a narrowing was ESTABLISHED, so + * every "cannot say" path answers `{ narrowed: false }` and the tool + * renders exactly what it rendered before: no security service in this + * deployment, a service predating the probe (feature-detected — the + * availability rule `ISecurityService` states at the top of its own + * file), or a throwing probe. ⛔ A diagnostic must never fail the read + * it annotates. + */ + diagnoseDelegation: async (object: string) => { + try { + // Typed against the published slot contract, never `any` + // (#4251): `Partial<…>` is the availability rule the contract + // itself states, and it is what makes the feature-detect below + // a TYPE-CHECKED narrowing rather than a property probe on an + // untyped value — the same shape `domains/automation.ts` uses. + const security = await deps.resolveService(context, 'security', envId) as + Partial | undefined; + if (typeof security?.describeDelegationNarrowing !== 'function') { + return { narrowed: false }; + } + return await security.describeDelegationNarrowing(object, ec); + } catch { + return { narrowed: false }; + } + }, get: async (object: string, id: string) => { const res: any = await callData('get', { object, id }, driver, envId, ec); return res?.record ?? res ?? null; diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index cfe48c185c..ad9e188d7d 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -75,6 +75,7 @@ "DefineSharingRuleInput (interface)", "DelegableAdminScope (interface)", "DelegableScope (interface)", + "DelegationNarrowing (type)", "DeployExecutionResult (interface)", "DriverQuery (type)", "EMBEDDER_SERVICE (const)", diff --git a/packages/spec/export-origins/contracts.json b/packages/spec/export-origins/contracts.json index e27f13eaab..0b58bb308a 100644 --- a/packages/spec/export-origins/contracts.json +++ b/packages/spec/export-origins/contracts.json @@ -75,6 +75,7 @@ "DefineSharingRuleInput": "src/contracts/sharing-service.ts#DefineSharingRuleInput (interface)", "DelegableAdminScope": "src/contracts/security-service.ts#DelegableAdminScope (interface)", "DelegableScope": "src/contracts/security-service.ts#DelegableScope (interface)", + "DelegationNarrowing": "src/contracts/security-service.ts#DelegationNarrowing (type)", "DeployExecutionResult": "src/contracts/deploy-pipeline-service.ts#DeployExecutionResult (interface)", "DriverQuery": "src/contracts/data-driver.ts#DriverQuery (type)", "EMBEDDER_SERVICE": "src/contracts/embedder.ts#EMBEDDER_SERVICE (const)", diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index 12fb1faac0..f60fc3b0d9 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -56,7 +56,7 @@ import type { FilterCondition } from '../data/filter.zod.js'; import type { ExecutionContext } from '../kernel/execution-context.zod.js'; import type { ExplainDecision, ExplainOperation } from '../security/explain.zod.js'; -import type { PermissionSet } from '../security/permission.zod.js'; +import type { ObjectAccessScope, PermissionSet } from '../security/permission.zod.js'; /** * The context shape these methods accept. @@ -191,6 +191,48 @@ export type AuthoredRowWriteVerdict = 'admit' | 'abstain'; */ export type AuthoredRowWriteOperation = 'update' | 'delete'; +/** + * [ADR-0090 D10 — maintainer ruling 2026-09-08] What + * {@link ISecurityService.describeDelegationNarrowing} reports about a + * delegated read. + * + * `narrowed: false` is the ONLY shape a non-delegated, system, principal-less + * or unresolvable context produces, and it carries no `statement` — so a + * consumer renders a note if and only if there is a narrowing to describe. A + * transport must not manufacture a warning from the absence of an answer: not + * knowing and knowing there is nothing to say are the same *rendered* outcome + * here on purpose, because the alternative is warning-fatigue on every read. + * + * A DISCRIMINATED UNION, not one shape with three optional fields, and the + * reason is `statement` itself: it is the sentence an AI consumer RENDERS, so + * left optional a consumer that forgets the `narrowed` check renders + * `undefined` — the same silence-by-omission this method exists to remove. The + * union makes the compiler enforce the invariant the prose above only asserts. + * The two shapes are also not symmetric under permanence: shipping this one and + * later LOOSENING it (a third member, or an optional field on the `true` arm) + * is non-breaking, while shipping optional fields and later TIGHTENING them to + * required is breaking — so the loose shape buys nothing and forecloses the + * tightening. + */ +export type DelegationNarrowing = + | { + /** No narrowing to describe. Carries nothing: there is nothing to say. */ + narrowed: false; + } + | { + /** The delegated principal's own ceiling narrowed the readable depth. */ + narrowed: true; + /** + * The sentence to surface. Written for an AI consumer: it states that the + * result is a SUBSET and that the count is not a fact about the object. + */ + statement: string; + /** The depth actually enforced for the delegated read. */ + effectiveScope: ObjectAccessScope; + /** The depth the delegator reaches alone. */ + delegatorScope: ObjectAccessScope; + }; + /** * Public contract for the `security` service. * @@ -465,7 +507,48 @@ export interface ISecurityService { resolveWriteScope( object: string, context?: SecurityContext, - ): Promise<'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'>; + ): Promise; + + /** + * [ADR-0090 D10 — maintainer ruling 2026-09-08] Did the D10 intersection + * NARROW what this delegated context can read on `object`, and if so, what + * should the caller be told? + * + * The diagnostic half of the delegated-read contract, and the reason it is a + * method rather than a caller-side derivation: the consumer being corrected + * is an **AI agent**. An MCP `query_records` answering `total: 0` with no note + * is read by the agent as a fact about the data, and it then tells a + * decision-maker "there are no opportunities this quarter" — the measured + * failure on issue #16549. A count served from a narrowed row set must + * therefore arrive WITH the narrowing stated, in the words the explain path + * already uses ("D10 intersection"), or the transport is lying by omission. + * + * `narrowed: true` iff the delegated principal's OWN sets declare a record + * DEPTH narrower than the delegator's for a read on `object` — the axis the + * ruling widened and the only axis this reports. It is resolved from the same + * evaluator calls the CRUD middleware stashes as `__readScope`, so it cannot + * claim a narrowing the query did not have, nor miss one it did. + * + * ⛔ What it deliberately does NOT report, because these are refusals rather + * than silent shrinkage and already surface as errors: an object the ceiling + * does not reach at all, a write refused on a read-only scope, a refused + * `allowTransfer`. And it reports nothing about narrowing that comes from the + * DELEGATOR's own grants — that is the user's own permissions working, which + * is exactly what the delegated path promises. + * + * **Non-delegated contexts answer `{ narrowed: false }`.** So does a system + * context, a principal-less one, and any internal failure: this is a + * diagnostic and must never fail, narrow, or block a read. + * + * **OPTIONAL.** A security service that predates it omits it and every + * consumer feature-detects (`typeof svc.describeDelegationNarrowing === + * 'function'`); absence reads as "cannot say", which callers render as no + * statement — the behaviour they had before this method existed. + */ + describeDelegationNarrowing?( + object: string, + context?: SecurityContext, + ): Promise; /** * [#5493 / ADR-0105 D3] Does an **app-authored** row-level security policy