Skip to content

Commit 8941669

Browse files
fix(runtime): the environment-membership check judges the principal the request resolved (#31396)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X1zG3Xn4JonaKvDLLr5gnjL)_ Refs #28284 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:多环境主机上,dispatcher 的环境成员检查(`403 PROJECT_MEMBERSHIP_REQUIRED`)现在判断的是请求解析出的主体。由 API key 或 OAuth access token 接纳的主体,按 key 的所有者或 token 的用户判断(同时带了别人 cookie 也一样),平台组织豁免使用解析时核验过的组织。由 session cookie 接纳的主体,判断方式与原来完全相同。 - **为什么改**:QA 第三轮发现的安全加固(细节按披露裁决不公开) - **风险与回滚**:只改 `@objectstack/runtime`(patch)。单环境主机,以及关闭了该检查的主机(`api.enforceProjectMembership: false`)不受影响。会被新拒绝的只有:多环境主机上,以 API key 或 OAuth token 访问某环境作用域路由、但不是该环境成员的调用方。回滚即 revert 本 PR。 - **先看**:`packages/runtime/src/http-dispatcher.ts` 的 `enforceProjectMembership`,session 读取之后按 `admittingCredentialKind` 新增的分支;以及新文件 `packages/runtime/src/security/admitting-credential.ts`。 - **两分钟验证**:`pnpm --filter @objectstack/runtime exec vitest run src/http-dispatcher-membership-resolved-principal.test.ts`,预期 8/8。 - **需要你做的**:无,席位做 contract review 后落地。 ## What changes - **New `admittingCredentialKind(request)`** (`packages/runtime/src/security/admitting-credential.ts`) is the one reading of which credential admitted a request's caller. - It answers `oauth` when the resolved context carries OAuth scopes. - Otherwise it answers what the identity step's admission recorded: `api_key` or `session`. - It answers `undefined` when nothing was recorded. - `apiForwardCredentialKind` (action execution) now delegates to it, so the api-action forward and the membership check cannot disagree. - **`HttpDispatcher.enforceProjectMembership`**: when that reading is `api_key` or `oauth`, the check judges the principal `resolveRequestScope` resolved. - That principal is `executionContext.userId`: the key's owner or the token's user. This holds even when a session cookie for another user rides beside it. - The platform-organization bypass reads `executionContext.tenantId`, the organization the resolution vetted. - **A session-admitted principal** keeps today's path exactly: the check's own session read, the session's active organization, and the fail-open when that read throws. - **Unchanged:** - hosts without environment scoping (no `kernel-resolver`, so no `environmentId`); - hosts that turn the check off; - the control-plane, share-link, approval-action and inbound-hook exemptions. ## Why QA 第三轮发现的安全加固(细节按披露裁决不公开) ## Verification Readings on head `44a099f05`: two commits on `main` at `c2bc76e3e`. The second answers the contract review: one shared credential reader, and a pin on a scoped data route. <details><summary>Pins, red-first and restore</summary> - New `packages/runtime/src/http-dispatcher-membership-resolved-principal.test.ts` (8): a multi-tenant host with a stub resolver and `sys_environment_member`, driven through `dispatch()`. - These are refused, and the refusal names the judged user: - a non-member admitted by an OAuth access token on `/mcp`; - the same token sent beside a member's cookie; - a non-member admitted by an API key on `/mcp`; - a non-member admitted by an API key on the scoped data route `/data/task`. - CONTROLs: - A cookie non-member is refused as before. - A member admitted by a token passes. So does a member admitted by a key. Each makes one membership read for that user. - A request with no credential is not judged here. - Red-first: with `http-dispatcher.ts` set back to its `main` blob, the file read 6 failed | 2 passed (8). - The four refusal cases failed. - The two member CONTROLs failed only because no membership read was made. - The cookie and no-credential CONTROLs stayed green. - Restored (blob equal to HEAD): 8 passed (8). </details> <details><summary>Package suite and typecheck</summary> - `tsc --noEmit` in `packages/runtime`: exit 0. - `pnpm --filter @objectstack/runtime test` under the verify lock: Test Files 556 passed (556), Tests 8965 passed | 19 skipped (8984), `VERDICT command-exit 0`. - An earlier cut of this change judged any resolved principal that differed from the check's own session read. This suite's `domains/meta-read-principal-filtering.measurement.test.ts` caught it, because the two reads can disagree for a session principal. - The arm is now keyed on the admitting credential, and that file is green. - The action-execution forward tests are green on the shared reader. </details> <details><summary>Derived gates</summary> - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derives 69 families on this diff. Every one ran and exited 0; `check:type-check-debt` ran under the verify lock. - `--ran` reconciliation: 69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN. - `eslint --no-inline-config` on the changed source files: exit 0. </details> Not measured: - An end-to-end multi-environment boot. No `kernel-resolver` ships in this repository, so the pins drive `dispatch()` with a stub resolver. - The dogfood battery. Its single-environment boots set no `environmentId`, so they never reach this check. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No `packages/spec` change.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `eslint --no-inline-config` on the changed files: exit 0. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English, except the maintainer summary the lane's PRs carry. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5mgePB9NQ4PNhvYVxk)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e13561 commit 8941669

5 files changed

Lines changed: 227 additions & 4 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): the environment-membership check judges the principal the request resolved
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
On a multi-environment host, the dispatcher refuses a caller who is not a member of the environment a scoped route resolves to (`403 PROJECT_MEMBERSHIP_REQUIRED`). That check now judges the principal the request resolved, whichever credential admitted it: a principal admitted by an OAuth access token on `/mcp` or by an API key is judged as that token's user or that key's owner, in the organization its resolution vetted, including when a session cookie for another user is sent beside it. A principal admitted by a session cookie is judged as before. Hosts without environment scoping, and hosts that turn the check off (`api.enforceProjectMembership: false`), are unchanged.

‎packages/runtime/src/action-execution.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ import { serveStoredMetadataRead, serveStoredMetadataReadsThrough } from './stor
8383
import { actorUserFromExecutionContext, resolveActorDisplayName } from './security/actor-user.js';
8484
import { actionInvocationBranch, actionInvocationFailure, actionInvocationTarget, recordActionInvocation, withActionSurface } from './action-invocation-audit.js';
8585
import type { HttpProtocolContext } from './http-dispatcher.js';
86+
import { admittingCredentialKind, type AdmittingCredentialKind } from './security/admitting-credential.js';
8687
import {
8788
GLOBAL_ACTION_OBJECT_KEY,
8889
actionHandlerObjectKeys,
@@ -1619,9 +1620,10 @@ function isIdentityRouteTarget(path: string): boolean {
16191620
* envelope carries its `oauthScopes`; such a token is honoured on the MCP
16201621
* endpoint alone), else what the door's admission recorded — `api_key` or
16211622
* `session`. `undefined` when the door recorded nothing (an anonymous caller,
1622-
* whom the door's own floor answers first).
1623+
* whom the door's own floor answers first). The one reading is
1624+
* {@link admittingCredentialKind}, which the environment-membership gate takes too.
16231625
*/
1624-
export type ApiForwardCredentialKind = 'session' | 'api_key' | 'oauth';
1626+
export type ApiForwardCredentialKind = AdmittingCredentialKind;
16251627

16261628
/** Which kind of door an api action's `target` is served by, for the forward. */
16271629
export type ApiForwardTargetDoor = 'identity' | 'mcp' | 'platform';
@@ -1710,8 +1712,7 @@ export function apiForwardAgentOf(requestContext: HttpProtocolContext | undefine
17101712

17111713
/** The kind of credential that admitted this request's caller (see {@link ApiForwardCredentialKind}). */
17121714
export function apiForwardCredentialKind(requestContext: HttpProtocolContext | undefined): ApiForwardCredentialKind | undefined {
1713-
if (Array.isArray(requestContext?.executionContext?.oauthScopes)) return 'oauth';
1714-
return requestContext?.admission?.credential;
1715+
return admittingCredentialKind(requestContext);
17151716
}
17161717

17171718
/**
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The environment-membership gate judges the principal the request resolved,
5+
* whichever credential admitted it.
6+
*
7+
* A multi-tenant host: a stub resolver places every request in one
8+
* environment, on a tenant kernel whose `sys_environment_member` holds a row
9+
* only for {@link MEMBER}. The gate's answer is read off the dispatch result
10+
* (`403 PROJECT_MEMBERSHIP_REQUIRED`, naming the user it judged) and off the
11+
* membership reads it made.
12+
*
13+
* - CONTROL: a cookie session for a non-member is refused, as before.
14+
* - A principal an OAuth access token admitted on `/mcp` is judged: refused
15+
* when it is not a member, and admitted when it is.
16+
* - A token for a non-member sent beside a member's cookie is judged as the
17+
* token's principal, the one the request resolved.
18+
* - A principal an API key admitted is judged as the key's owner, on `/mcp`
19+
* and on a scoped data route alike.
20+
*/
21+
22+
import { describe, it, expect, vi } from 'vitest';
23+
// Paid at collection: the dispatcher reaches it through a dynamic import.
24+
import '@objectstack/metadata';
25+
import { hashApiKey } from '@objectstack/core';
26+
import { HttpDispatcher } from './http-dispatcher.js';
27+
import type { HttpProtocolContext, KernelResolver } from './http-dispatcher.js';
28+
29+
const ENV = 'env-tenant';
30+
const MEMBER = 'user-member';
31+
const OUTSIDER = 'user-outsider';
32+
/** JWT-shaped: three dot-separated segments, which is what the token door reads. */
33+
const TOKEN = 'eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ4In0.c2ln';
34+
const API_KEY = 'osk_probe0123456789abcdefghijklmnopqrstuvwxyz';
35+
const MCP_CALL = { jsonrpc: '2.0', id: 1, method: 'tools/list' };
36+
37+
function kernelWith(services: Record<string, unknown>): any {
38+
return {
39+
getState: () => 'running',
40+
getService: (n: string) => services[n] ?? null,
41+
getServiceAsync: async (n: string) => services[n] ?? null,
42+
context: { getService: (n: string) => services[n] ?? null },
43+
};
44+
}
45+
46+
/**
47+
* The tenant host. `cookieUser` is who the cookie session reads as (none when
48+
* absent); `tokenUser` is who a verified OAuth access token names.
49+
*/
50+
function tenantHost(opts: { cookieUser?: string; tokenUser?: string; keyOwner?: string }) {
51+
const find = vi.fn(async (object: string, query: { where?: Record<string, unknown> }) => {
52+
const where = query?.where ?? {};
53+
if (object === 'sys_environment_member') {
54+
return where.environment_id === ENV && where.user_id === MEMBER
55+
? [{ id: 'm1', environment_id: ENV, user_id: MEMBER }]
56+
: [];
57+
}
58+
if (object === 'sys_api_key' && opts.keyOwner && where.key === hashApiKey(API_KEY)) {
59+
return [{ id: 'key-1', key: hashApiKey(API_KEY), user_id: opts.keyOwner }];
60+
}
61+
if (object === 'sys_user' && typeof where.id === 'string') return [{ id: where.id }];
62+
return [];
63+
});
64+
const objectql = {
65+
find,
66+
getObjects: vi.fn().mockReturnValue({}),
67+
registry: {
68+
getObject: vi.fn((name: string) => (name === 'sys_environment_member' ? { name } : null)),
69+
getRegisteredTypes: vi.fn().mockReturnValue([]),
70+
},
71+
};
72+
const auth = {
73+
getApi: async () => ({
74+
getSession: async () => (opts.cookieUser
75+
? { user: { id: opts.cookieUser }, session: { userId: opts.cookieUser } }
76+
: null),
77+
}),
78+
verifyMcpAccessToken: vi.fn(async () => (opts.tokenUser
79+
? { userId: opts.tokenUser, clientId: 'client-1', scopes: ['data:read'] }
80+
: null)),
81+
};
82+
const tenant = kernelWith({ objectql, auth });
83+
const resolver: KernelResolver = {
84+
resolveKernel: async (c: HttpProtocolContext) => {
85+
c.environmentId = ENV;
86+
return tenant;
87+
},
88+
};
89+
const dispatcher = new HttpDispatcher(kernelWith({}), undefined, {
90+
kernelResolver: resolver,
91+
enforceProjectMembership: true,
92+
});
93+
const membershipReads = () => find.mock.calls
94+
.filter((call) => call[0] === 'sys_environment_member')
95+
.map((call) => (call[1] as { where: { user_id: string } }).where.user_id);
96+
return { dispatcher, membershipReads };
97+
}
98+
99+
const mcp = (headers: Record<string, string>): HttpProtocolContext => ({
100+
request: new Request('http://tenant.example/api/v1/mcp', { method: 'POST', headers }),
101+
});
102+
103+
function membershipRefusal(result: { response?: { status: number; body?: any } }) {
104+
const error = result.response?.body?.error;
105+
return result.response?.status === 403 && error?.code === 'PROJECT_MEMBERSHIP_REQUIRED'
106+
? { userId: error?.details?.userId as string | undefined }
107+
: undefined;
108+
}
109+
110+
describe('the environment-membership gate judges the resolved principal', () => {
111+
it('CONTROL: a cookie session for a non-member is refused', async () => {
112+
const { dispatcher, membershipReads } = tenantHost({ cookieUser: OUTSIDER });
113+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({}));
114+
expect(membershipRefusal(result)).toEqual({ userId: OUTSIDER });
115+
expect(membershipReads()).toEqual([OUTSIDER]);
116+
});
117+
118+
it('a principal an OAuth access token admitted, not a member: refused, naming the token\'s user', async () => {
119+
const { dispatcher, membershipReads } = tenantHost({ tokenUser: OUTSIDER });
120+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({ authorization: `Bearer ${TOKEN}` }));
121+
expect(membershipRefusal(result), JSON.stringify(result.response?.body)).toEqual({ userId: OUTSIDER });
122+
expect(membershipReads()).toEqual([OUTSIDER]);
123+
});
124+
125+
it('a token for a non-member beside a member\'s cookie: judged as the token\'s principal, refused', async () => {
126+
const { dispatcher, membershipReads } = tenantHost({ cookieUser: MEMBER, tokenUser: OUTSIDER });
127+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({ authorization: `Bearer ${TOKEN}` }));
128+
expect(membershipRefusal(result), JSON.stringify(result.response?.body)).toEqual({ userId: OUTSIDER });
129+
expect(membershipReads()).toEqual([OUTSIDER]);
130+
});
131+
132+
it('a principal an API key admitted, not a member: refused, naming the key\'s owner', async () => {
133+
const { dispatcher, membershipReads } = tenantHost({ keyOwner: OUTSIDER });
134+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({ 'x-api-key': API_KEY }));
135+
expect(membershipRefusal(result), JSON.stringify(result.response?.body)).toEqual({ userId: OUTSIDER });
136+
expect(membershipReads()).toEqual([OUTSIDER]);
137+
});
138+
139+
it('a principal an API key admitted on a scoped non-MCP route, not a member: refused, naming the key\'s owner', async () => {
140+
const { dispatcher, membershipReads } = tenantHost({ keyOwner: OUTSIDER });
141+
const context: HttpProtocolContext = {
142+
request: new Request('http://tenant.example/api/v1/data/task', { headers: { 'x-api-key': API_KEY } }),
143+
};
144+
const result = await dispatcher.dispatch('GET', '/data/task', undefined, {}, context);
145+
expect(membershipRefusal(result), JSON.stringify(result.response?.body)).toEqual({ userId: OUTSIDER });
146+
expect(membershipReads()).toEqual([OUTSIDER]);
147+
});
148+
149+
it('CONTROL: a principal an OAuth access token admitted, a member: passes the gate', async () => {
150+
const { dispatcher, membershipReads } = tenantHost({ tokenUser: MEMBER });
151+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({ authorization: `Bearer ${TOKEN}` }));
152+
expect(membershipRefusal(result)).toBeUndefined();
153+
expect(membershipReads()).toEqual([MEMBER]);
154+
});
155+
156+
it('CONTROL: a principal an API key admitted, a member: passes the gate', async () => {
157+
const { dispatcher, membershipReads } = tenantHost({ keyOwner: MEMBER });
158+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({ 'x-api-key': API_KEY }));
159+
expect(membershipRefusal(result)).toBeUndefined();
160+
expect(membershipReads()).toEqual([MEMBER]);
161+
});
162+
163+
it('CONTROL: no credential at all is not judged here', async () => {
164+
const { dispatcher, membershipReads } = tenantHost({});
165+
const result = await dispatcher.dispatch('POST', '/mcp', MCP_CALL, {}, mcp({}));
166+
expect(membershipRefusal(result)).toBeUndefined();
167+
expect(membershipReads()).toEqual([]);
168+
});
169+
});

‎packages/runtime/src/http-dispatcher.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import { DomainHandlerRegistry, type DomainRoute, type DomainHandlerDeps } from
3838
// only — `actionExecutionDeps` hands the endpoint step the same `deps` object
3939
// every domain already passes to `callData`, without this file calling it.
4040
import type { ActionExecutionDeps } from './action-execution.js';
41+
import { admittingCredentialKind } from './security/admitting-credential.js';
4142
import { createAnalyticsDomain, handleAnalyticsRequest } from './domains/analytics.js';
4243
import { isServiceServeable } from './service-serveable.js';
4344
import { createI18nDomain, handleI18nRequest } from './domains/i18n.js';
@@ -1673,6 +1674,7 @@ export class HttpDispatcher {
16731674
// and single-tenant setups run without auth.
16741675
let userId: string | undefined;
16751676
let activeOrganizationId: string | undefined;
1677+
let sessionUnread = false;
16761678
try {
16771679
// [#4127 batch 4] This read was `authService?.api?.getSession?.(…)`
16781680
// with NO `getApi()` fallback — the only one of the three `.api`
@@ -1698,6 +1700,22 @@ export class HttpDispatcher {
16981700
userId = sessionData?.user?.id ?? sessionData?.session?.userId;
16991701
activeOrganizationId = sessionData?.session?.activeOrganizationId;
17001702
} catch {
1703+
sessionUnread = true;
1704+
}
1705+
1706+
// The gate judges the principal this request RESOLVED
1707+
// (`resolveRequestScope`), whichever credential admitted it. A
1708+
// principal an API key or an OAuth access token admitted (with or
1709+
// without a cookie beside it) is the one judged, in the organization
1710+
// its resolution vetted; which credential that was is the one reading
1711+
// the api-action forward takes too (`admittingCredentialKind`). A
1712+
// session principal is judged by the session read above, as before.
1713+
const resolved = context.executionContext;
1714+
const admittedBy = admittingCredentialKind(context);
1715+
if ((admittedBy === 'api_key' || admittedBy === 'oauth') && resolved?.userId) {
1716+
userId = resolved.userId;
1717+
activeOrganizationId = resolved.tenantId;
1718+
} else if (sessionUnread) {
17011719
// Auth resolution failed — do not block the request on RBAC.
17021720
return null;
17031721
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Which kind of credential admitted a request's caller: the one reading the
5+
* dispatcher's environment-membership gate and the api-action forward both
6+
* take, so the two cannot disagree.
7+
*
8+
* - `oauth` when the principal was resolved from an OAuth access token (the
9+
* envelope carries its `oauthScopes`; such a token is honoured on the MCP
10+
* endpoint alone, and an in-process forward of it carries them too);
11+
* - else what the identity step's admission recorded: `api_key` or `session`;
12+
* - `undefined` when it recorded nothing (an anonymous caller).
13+
*/
14+
export type AdmittingCredentialKind = 'session' | 'api_key' | 'oauth';
15+
16+
/** The two request facts {@link admittingCredentialKind} reads. */
17+
export interface AdmittingCredentialFacts {
18+
executionContext?: { oauthScopes?: unknown };
19+
admission?: { credential: 'api_key' | 'session' };
20+
}
21+
22+
export function admittingCredentialKind(request: AdmittingCredentialFacts | undefined): AdmittingCredentialKind | undefined {
23+
if (Array.isArray(request?.executionContext?.oauthScopes)) return 'oauth';
24+
return request?.admission?.credential;
25+
}

0 commit comments

Comments
 (0)