Skip to content

Commit 331a1a2

Browse files
os-teslaclaude
andauthored
fix(security): OAuth-connected MCP agents run at the delegator's recorded scope, and a narrowed delegated read says so (#17332)
* fix(security): OAuth MCP agent runs at the delegator's record depth (ADR-0090 D10) WIP checkpoint before build/test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * fix(security): a silent agent ceiling subtracts no depth (ADR-0090 D10, #16549) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * fix(security): OAuth MCP agent reads at the delegator's depth; narrowed reads say so Ruling batch #81 item 1 (option 1). A silent agent ceiling declares no record depth, so the D10 intersection takes the delegator's; the ceiling keeps every other subtraction, written down beside the sets. MCP query_records carries the D10 statement when a delegated read was narrowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * chore: regenerate the spec surface ledgers and the system-context census The published-surface delta this card adds, measured by the repo's own tools: `@objectstack/spec` ./contracts gains one entry, `DelegationNarrowing (interface)` — 0 breaking, 1 added. The census gains one elevation read (the diagnostic's `context.isSystem` guard). Types the runtime's new security-slot lookup against `Partial<ISecurityService>` so the slot-lookup ratchet stays at its baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(dogfood): the #16549 measured table, end to end on the served showcase Human leg over the REST door with a real admin token vs the OAuth agent leg through the engine with the producer's own context — same person, same object. Plus the control that the widening reaches a viewAllRecords profile and nobody else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * chore: regenerate the system-context census after the main sync The census page's six declared counts are written by `pnpm gen:system-context-census`, never by hand. Both sides of the merge had independently raised them 108 -> 109 for their own new elevation read, so the merge kept one 109 while the union is 110 — the mechanical `[declared-count]` mismatch the gate reports. Regenerated wholesale after the merge commit: headline-sites, sharing-total, table-reads, table-elevation-reads, table-behaviour-bearing and ruling-sites, each +1. No anchor rewrite (this page carries no line numbers) and no human-edit-class problem was reported. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * refactor(spec): make DelegationNarrowing a discriminated union on `narrowed` The three fields were documented as "present only when `narrowed`" while the type left them optional, so every consumer had to read the discriminant and then non-null-assert three fields the compiler still believed might be absent. The invariant lived in prose, where nothing enforced it. `statement` is the sentence an AI consumer renders. Left optional, a consumer that forgets the `narrowed` check renders `undefined` — the same silence this diagnostic exists to remove. The union makes the compiler enforce it. The two shapes are also asymmetric under permanence: shipping optional and later tightening to required is breaking, while shipping discriminated and later loosening (a new member, or an optional field on the `true` arm) is not. The five-member scope ladder the type spelled inline twice — and `resolveWriteScope` a third time — now names the alias that already exists for it: `ObjectAccessScope` (`security/permission.zod.ts`, ADR-0057 D1), the Zod enum the permission-set authoring schema itself uses. Same five members, so no narrowing; zero new exported names. Same information, same runtime values. Baselines regenerated with the repo's own generators. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * docs(changeset): record the union shape and the reused scope alias The body called `DelegationNarrowing` a new exported interface; it is now a discriminated union, so the word was wrong. States the permanence asymmetry that decides the shape, and that the five-member ladder names the existing `ObjectAccessScope` rather than a second declaration of one ladder — so the published surface still gains exactly one name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * docs(changeset): state the release-direction asymmetry as a table The prose spelled the cost of one direction as a bold-emphasised word, which is byte-identical to this repo's breaking-change declaration marker, so `check-adr-0087-registration` read the changeset as declaring a breaking change and asked for an ADR-0087 disposition. There is none to give: this PR is `minor` and purely additive — the api-surface baseline gains one line and removes none, and the sentence was describing a hypothetical FUTURE direction, not this change. A table says the same thing without minting the marker. ⛔ Not an evasion of a real declaration: nothing here is breaking, so no disposition is owed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e958468 commit 331a1a2

14 files changed

Lines changed: 891 additions & 24 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
"@objectstack/spec": minor
4+
"@objectstack/mcp": minor
5+
"@objectstack/runtime": minor
6+
---
7+
8+
fix(security): an OAuth-connected MCP agent runs at its delegator's record depth — "you connect as yourself" becomes true (#16549)
9+
10+
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.**
11+
12+
**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:
13+
14+
| identity path | `crm_account` | `crm_opportunity` | `crm_task` |
15+
|:--|--:|--:|--:|
16+
| API key, `principalKind: human` | 9 | 23 | 45 |
17+
| OAuth, `principalKind: agent`, `onBehalfOf` = same user | **5** | **0** | **0** |
18+
19+
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.
20+
21+
**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.
22+
23+
**(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.
24+
25+
⛔ **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.
26+
27+
**(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.
28+
29+
**(3)** The Setup page's promise is untouched — it is now true rather than rewritten.
30+
31+
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.
32+
33+
`DelegationNarrowing` is a **discriminated union** on `narrowed`, not one shape with three optional fields, because the two shapes are not symmetric once released:
34+
35+
| direction, after release | consumer cost |
36+
|:--|:--|
37+
| ship optional fields, later tighten them to required | a compile break |
38+
| ship discriminated, later loosen it (a new union member, or an optional field on the `true` arm) | none |
39+
40+
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.

‎content/docs/permissions/system-context.mdx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a
99
service self-write, a migration.
1010

1111
This page is **the authority** for what that flag actually does. It exists
12-
because the flag is not one concept: it is a single boolean read at **109
12+
because the flag is not one concept: it is a single boolean read at **110
1313
distinct sites across 20 packages**, and knowing three of those behaviours gives
1414
no hint that the other hundred-and-four exist. Every documented app-side bug
1515
traced to `isSystem` had the same shape — the metadata was complete and correct,
@@ -133,7 +133,7 @@ that silently does not happen.
133133

134134
### 3. Sharing (`plugin-sharing`)
135135

136-
The largest single consumer — **17 of the 109 sites**.
136+
The largest single consumer — **17 of the 110 sites**.
137137

138138
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
139139
|:--|:---|:---|:---|
@@ -279,7 +279,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are
279279
independent decisions, and a seed loader plausibly wants the first two but not
280280
the third. The concept is nevertheless **staying as one boolean**:
281281

282-
- **Shipped semantics.** `isSystem` is a published contract with 109 read sites
282+
- **Shipped semantics.** `isSystem` is a published contract with 110 read sites
283283
in 20 packages. Splitting it is a breaking contract change across all of them.
284284
(The ruling was taken when the census read 80 sites in 18 packages; the count
285285
has grown, which strengthens rather than weakens the argument.)
@@ -353,12 +353,12 @@ still holds equal to the census on every pull request:
353353
| Appearances of the bare identifier `isSystem` in non-test sources | 813 | — |
354354
| — parsed as a declaration | 23 | ✅ |
355355
| — parsed as an object-literal / type key (producers and option objects) | 310 | — |
356-
| — parsed as a property **read** | 115 | ✅ |
356+
| — parsed as a property **read** | 116 | ✅ |
357357
| — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ |
358358
| — the remainder: text inside comments and string literals | 358 | — |
359359
| Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ |
360-
| Of those reads: reads of `ExecutionContext.isSystem` | **109** | ✅ |
361-
| — behaviour-bearing (rows 1–63 above) | 105 | ✅ |
360+
| Of those reads: reads of `ExecutionContext.isSystem` | **110** | ✅ |
361+
| — behaviour-bearing (rows 1–63 above) | 106 | ✅ |
362362
| — carry the flag onward only (rows 64–67 above) | 4 | ✅ |
363363
| Packages containing at least one elevation read | **20** | ✅ |
364364
| Files containing at least one elevation read | 45 | ✅ |

‎packages/mcp/src/mcp-http-tools.ts‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,29 @@ export interface McpDataBridge {
9494
orderBy?: Array<{ field: string; order: 'asc' | 'desc' }>;
9595
},
9696
): Promise<unknown>;
97+
/**
98+
* [ADR-0090 D10 — maintainer ruling 2026-09-08, #16549] Is a read on `object`
99+
* NARROWED by the agent ceiling the caller runs under?
100+
*
101+
* The transport half of the ruling's consequence (2), and it exists because
102+
* the deceived consumer on this surface is the **AI itself**: a delegated
103+
* `query_records` answering `total: 0` with no note is read by the agent as a
104+
* fact about the data, and it then tells a decision-maker "there are no
105+
* opportunities this quarter". A narrowed count must arrive WITH the
106+
* narrowing stated.
107+
*
108+
* OPTIONAL, with the same graceful-degradation contract as
109+
* {@link McpDataBridge.listObjectsDiagnosed} and
110+
* {@link McpDataBridge.aggregate}: a bridge bound to a principal that cannot
111+
* be delegated (the stdio API-key host), or wired to a security service
112+
* predating the probe, omits this member and the tool renders exactly what it
113+
* rendered before.
114+
*
115+
* ⛔ `{ narrowed: false }` and "cannot say" are deliberately the same RENDERED
116+
* outcome — neither may manufacture a warning. The tool states a narrowing
117+
* only where one was established.
118+
*/
119+
diagnoseDelegation?(object: string): Promise<{ narrowed: boolean; statement?: string } | undefined>;
97120
get(object: string, id: string): Promise<unknown>;
98121
create(object: string, data: Record<string, unknown>): Promise<unknown>;
99122
update(object: string, id: string, data: Record<string, unknown>): Promise<unknown>;
@@ -231,6 +254,26 @@ function errorResult(message: string) {
231254
return { content: [{ type: 'text' as const, text: message }], isError: true as const };
232255
}
233256

257+
/**
258+
* [ADR-0090 D10 — ruling 2026-09-08, consequence 2] Attach the D10 narrowing
259+
* notice to a query result WITHOUT reshaping it.
260+
*
261+
* The bridge's `query` is typed `Promise<unknown>`, and the shape that ships is
262+
* the protocol's `{ object, records, total }` — so the object case adds two
263+
* keys beside the existing ones, mirroring `list_objects`' `partial` / `warning`
264+
* pair so a client branches on the same vocabulary on both tools. Anything that
265+
* is NOT a plain object (a bare array from some future bridge) keeps its own
266+
* shape and carries the notice alongside: inventing a `records` wrapper there
267+
* would break a caller to deliver a warning about breaking callers.
268+
*/
269+
function withDelegationNotice(value: unknown, statement: string): unknown {
270+
const notice = { delegationNarrowed: true as const, warning: statement };
271+
if (value !== null && typeof value === 'object' && !Array.isArray(value)) {
272+
return { ...(value as Record<string, unknown>), ...notice };
273+
}
274+
return { result: value, ...notice };
275+
}
276+
234277
function jsonText(value: unknown): string {
235278
try {
236279
return JSON.stringify(value, null, 2);
@@ -494,7 +537,19 @@ export function registerObjectTools(
494537
offset,
495538
orderBy,
496539
});
497-
return textResult(result);
540+
// [ADR-0090 D10 — ruling 2026-09-08, consequence 2] A delegated read
541+
// the agent ceiling NARROWED is served with the narrowing stated. The
542+
// rows are still served — a partial answer is the most useful true
543+
// thing here, exactly as in `list_objects` above; what is withheld is
544+
// the implicit claim that this count describes the object.
545+
//
546+
// ⛔ The probe may never fail the query it annotates: it is caught
547+
// into "no statement", which is byte-for-byte the pre-#16549 render.
548+
const diagnosed = typeof bridge.diagnoseDelegation === 'function'
549+
? await bridge.diagnoseDelegation(objectName).catch(() => undefined)
550+
: undefined;
551+
if (!diagnosed?.narrowed || !diagnosed.statement) return textResult(result);
552+
return textResult(withDelegationNotice(result, diagnosed.statement));
498553
} catch (err) {
499554
return errorResult(messageOf(err));
500555
}
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [ADR-0090 D10 — maintainer ruling 2026-09-08, #16549 consequence 2]
5+
* A delegated `query_records` NARROWED by the agent ceiling says so; an
6+
* un-narrowed one does not.
7+
*
8+
* This is the half of the ruling that is INDEPENDENT of the parity half, and
9+
* the half that matters most, because the deceived consumer on this surface is
10+
* the AI itself. Measured on the card: `query_records` answered `total: 0` with
11+
* no note, and the agent faithfully told a decision-maker "there are no
12+
* opportunities this quarter" — a wrong answer delivered with full confidence,
13+
* from a tool that was working exactly as specified.
14+
*
15+
* The rows are still served. What the notice adds is the one fact the payload
16+
* could not previously carry: this count describes the CEILING, not the object.
17+
* The shape mirrors `list_objects`' `partial` / `warning` pair (#6504) so a
18+
* client branches on the same vocabulary across both tools.
19+
*/
20+
21+
import { describe, it, expect, beforeEach } from 'vitest';
22+
23+
import { MCPServerRuntime } from './mcp-server-runtime.js';
24+
import type { McpDataBridge } from './mcp-http-tools.js';
25+
26+
const NARROWING = 'This result was narrowed by the ADR-0090 D10 intersection: rows are ABSENT from this result.';
27+
28+
function makeBridge(
29+
diagnose?: McpDataBridge['diagnoseDelegation'],
30+
): McpDataBridge & { calls: string[] } {
31+
const calls: string[] = [];
32+
const bridge: any = {
33+
calls,
34+
async listObjects() { return [{ name: 'crm_opportunity', label: 'Opportunity' }]; },
35+
async describeObject(name: string) { return { name }; },
36+
async query(object: string) {
37+
calls.push('query');
38+
return { object, records: [], total: 0 };
39+
},
40+
async get(object: string, id: string) { return { object, id }; },
41+
async create(object: string, data: any) { return { object, data }; },
42+
async update(object: string, id: string) { return { object, id }; },
43+
async remove(object: string, id: string) { return { object, id, success: true }; },
44+
};
45+
if (diagnose) bridge.diagnoseDelegation = diagnose;
46+
return bridge;
47+
}
48+
49+
async function queryRecords(runtime: MCPServerRuntime, bridge: any): Promise<any> {
50+
const body = {
51+
jsonrpc: '2.0',
52+
id: 1,
53+
method: 'tools/call',
54+
params: { name: 'query_records', arguments: { objectName: 'crm_opportunity' } },
55+
};
56+
const req = new Request('http://localhost/api/v1/mcp', {
57+
method: 'POST',
58+
headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' },
59+
body: JSON.stringify(body),
60+
});
61+
const res = await runtime.handleHttpRequest(req, { bridge, parsedBody: body });
62+
const json: any = await res.json();
63+
expect(json.result?.isError).toBeFalsy();
64+
return JSON.parse(json.result.content[0].text);
65+
}
66+
67+
describe('query_records — ADR-0090 D10 delegated-read diagnostic (#16549)', () => {
68+
let runtime: MCPServerRuntime;
69+
70+
beforeEach(() => {
71+
runtime = new MCPServerRuntime({ name: 't', version: '1.0.0' });
72+
});
73+
74+
it('a NARROWED delegated read carries the D10 statement beside the rows', async () => {
75+
const bridge = makeBridge(async () => ({ narrowed: true, statement: NARROWING }));
76+
const payload = await queryRecords(runtime, bridge);
77+
// The rows are still served — a partial answer is the most useful true
78+
// thing here. What is added is the withheld claim.
79+
expect(payload.object).toBe('crm_opportunity');
80+
expect(payload.records).toEqual([]);
81+
expect(payload.total).toBe(0);
82+
expect(payload.delegationNarrowed).toBe(true);
83+
expect(payload.warning).toBe(NARROWING);
84+
});
85+
86+
it('an UN-NARROWED delegated read carries NO statement — the payload is byte-identical to before', async () => {
87+
const bridge = makeBridge(async () => ({ narrowed: false }));
88+
const payload = await queryRecords(runtime, bridge);
89+
expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 });
90+
expect(payload.delegationNarrowed).toBeUndefined();
91+
expect(payload.warning).toBeUndefined();
92+
});
93+
94+
it('a bridge that cannot answer (no member at all) renders exactly what it rendered before', async () => {
95+
// The stdio API-key host and any deployment whose security service predates
96+
// the probe. ⛔ Absence must never be rendered as a warning — "cannot say"
97+
// and "nothing to say" are deliberately the same rendered outcome.
98+
const bridge = makeBridge(undefined);
99+
const payload = await queryRecords(runtime, bridge);
100+
expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 });
101+
});
102+
103+
it('a THROWING probe never fails the query it annotates', async () => {
104+
const bridge = makeBridge(async () => { throw new Error('security service exploded'); });
105+
const payload = await queryRecords(runtime, bridge);
106+
expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 });
107+
expect(bridge.calls).toContain('query');
108+
});
109+
110+
it('narrowed:true with NO statement adds nothing — a notice with no sentence is not a notice', async () => {
111+
const bridge = makeBridge(async () => ({ narrowed: true }));
112+
const payload = await queryRecords(runtime, bridge);
113+
expect(payload).toEqual({ object: 'crm_opportunity', records: [], total: 0 });
114+
});
115+
});

0 commit comments

Comments
 (0)