Skip to content

Commit 8e13ca8

Browse files
baozhoutaoclaude
andauthored
fix(plugin-sharing): share-link enforcement takes the whole authz envelope (#6206) (#6552)
The share-link routes rebuilt a four-field object out of the `resolveAuthzContext` result (`userId`/`tenantId`/`positions`/`permissions`) and handed it straight to `engine.find` as the [Finding-2] visibility check's context. `accessible_org_ids`, `org_user_ids`, `systemPermissions`, `posture` and `tabPermissions` were dropped on the way into enforcement. Under the `group` tenancy posture `accessible_org_ids` IS the Layer 0 wall (ADR-0105 D2) and an absent set denies, so the check failed closed and link creation answered 403 for records the caller reads fine elsewhere — reproduced here, not only read from the code. The envelope is now passed through whole (`{ ...authz, isSystem: false }`), per the maintainer's option-A ruling on #6206 and the contract half that landed with #6511. `posture` travels with the context and is never re-derived at the enforcement site (ADR-0095 D2). `ShareLinkExecutionContext` survives as the routes' own 401 vocabulary, consumed only by the new `isAuthenticated` gate. Tests: a seam-parity pin in plugin-sharing (the enforcement context must carry every key the real resolver produced — re-trimming fails by naming the dropped keys) and the behavioural `group`-posture repro in plugin-security, which owns `computeTenantLayer0Filter` and can therefore drive the real wall. Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv Co-authored-by: Claude <noreply@anthropic.com>
1 parent f549a0d commit 8e13ca8

6 files changed

Lines changed: 689 additions & 22 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/plugin-sharing": patch
3+
---
4+
5+
fix(plugin-sharing): share-link 路由把完整授权信封交给 enforcement,修复 `group` 姿态下建链恒 403(#6206,裁决 A 案的消费半边)
6+
7+
`SharingServicePlugin` 的 share-link 路由此前在 `resolveAuthzContext` 之后重新
8+
拼一个四字段对象(`userId` / `tenantId` / `positions` / `permissions`),而这个
9+
对象被原样当作 enforcement context 喂进 `engine.find` —— 即 [Finding-2]
10+
「只能为你自己看得见的记录建链接」那道可见性校验。被丢在半路的是
11+
`accessible_org_ids``org_user_ids``systemPermissions``posture`
12+
`tabPermissions`
13+
14+
实害(已复现,非仅代码读出):`group` 租户姿态下 `accessible_org_ids` 就是
15+
Layer 0 那堵墙(ADR-0105 D2),集合缺席即判否(fail closed)。于是可见性校验
16+
查不到任何行,建链接对**调用方本来读得到的记录**返回
17+
`403 FORBIDDEN: Not permitted to share <object>/<id>` —— 一个已发布姿态上,
18+
已发布功能完全不可用。`single` 姿态(默认)不读该字段,行为不变。
19+
20+
改法按维护者 2026-08-07 的 A 案裁决(契约半边 #6430 / PR #6511 已落):信封
21+
**整个**透传(`{ ...authz, isSystem: false }`),不再逐字段挑选 —— 逐字段挑选正是
22+
这条缝出问题的方式,也是下一个新增授权维度会再次漏掉的地方。`posture` 随上下文
23+
流动、不在 enforcement 处重推(ADR-0095 D2)。窄类型 `ShareLinkExecutionContext`
24+
保留,但只服务路由自己的 401 判定(认证与否),不再出现在任何裁决路径上。
25+
26+
`ShareLinkService.createLink` / `revokeLink` / `listLinks``canManageShares`
27+
探针的参数类型随之收成完整 `ExecutionContext`,与 #6511 落地的契约一致。
Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#6206 / #6430 ruling A] The `group`-posture repro: minting a share link for
5+
* a record the caller can read.
6+
*
7+
* ## Why this file lives in plugin-SECURITY
8+
*
9+
* The defect is a seam in `@objectstack/plugin-sharing` (its share-link routes
10+
* rebuilt a four-field subset of the `resolveAuthzContext` envelope and fed it
11+
* to `engine.find` as the [Finding-2] visibility check's context), but the
12+
* VERDICT that made it a 403 is computed here: `computeTenantLayer0Filter`
13+
* reads `ExecutionContext.accessible_org_ids` and, under the `group` posture,
14+
* an absent/empty set denies (ADR-0105 D2, fail closed). Proving the bug
15+
* therefore needs both packages in one process, and this is the one that owns
16+
* the wall — plugin-security already depends on plugin-sharing for the same
17+
* reason (`controlled-by-parent-sharing.test.ts`,
18+
* `vama-write-path-convergence.test.ts`), never the other way round.
19+
*
20+
* ## What is real here and what is a double
21+
*
22+
* REAL: the plugin's own route wiring and context assembly (the plugin is
23+
* booted, so the closure under test is the production one), the share-link
24+
* service, and the tenant wall — `computeTenantLayer0Filter` is called with the
25+
* context the route actually produced, exactly as `security-plugin.ts` calls it
26+
* on a read.
27+
*
28+
* DOUBLE: storage. The engine below is an in-memory table set that applies the
29+
* wall the same way the security middleware does — AND-composed first, on a
30+
* non-system context — so `RLS_DENY_FILTER` denies by being an unmatchable
31+
* predicate rather than by a special case, which is how it denies in
32+
* production.
33+
*
34+
* ## Before/after, recorded
35+
*
36+
* With the four-field assembly restored in plugin-sharing, `groupPostureMint`
37+
* answers 403 (`FORBIDDEN: Not permitted to share crm_account/acc_1`) — the
38+
* card's repro — while the `single`-posture case stays 201. After the fix the
39+
* `group` case is 201 and the `single` case is unchanged. The third case is the
40+
* one that keeps the fix honest: a caller with no membership in the record's
41+
* organization must STILL be refused, because the envelope was widened, not the
42+
* authority.
43+
*/
44+
45+
import { describe, it, expect, vi } from 'vitest';
46+
// The producers' OWN dispatch predicates for the double's write verbs, from
47+
// `@objectstack/metadata-core` (where they live since #5619) — this package
48+
// does not depend on `@objectstack/objectql`, and taking that edge to reach the
49+
// re-export would be a cycle turbo refuses.
50+
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core';
51+
import type { TenancyPosture } from '@objectstack/spec/security';
52+
import { SharingServicePlugin } from '@objectstack/plugin-sharing';
53+
import { computeTenantLayer0Filter } from './tenant-layer.js';
54+
55+
const BASE = '/api/v1/share-links';
56+
const OBJECT = 'crm_account';
57+
const RECORD = 'acc_1';
58+
const ORG_A = 'org_plant_a';
59+
const ORG_B = 'org_plant_b';
60+
61+
/** Objects that carry `organization_id` — the wall's "is this a tenant object?" input. */
62+
const TENANT_OBJECTS = new Set([OBJECT]);
63+
64+
function matches(row: any, where: Record<string, any>): boolean {
65+
return Object.entries(where).every(([k, v]) => {
66+
if (v && typeof v === 'object' && '$in' in v) return (v as any).$in.includes(row[k]);
67+
return row[k] === v;
68+
});
69+
}
70+
71+
/**
72+
* An engine that enforces Layer 0 exactly as the security middleware does: the
73+
* REAL `computeTenantLayer0Filter`, fed the caller's context, AND-composed onto
74+
* the query's own predicate. A system context bypasses it, as it does in
75+
* production.
76+
*/
77+
function makeEngine(tables: Record<string, any[]>, posture: TenancyPosture) {
78+
return {
79+
async find(object: string, opts: any) {
80+
const ctx = opts?.context ?? {};
81+
let rows = tables[object] ?? [];
82+
if (!ctx.isSystem && TENANT_OBJECTS.has(object)) {
83+
const layer0 = computeTenantLayer0Filter({
84+
tenancyPosture: posture,
85+
organizationId: ctx.tenantId,
86+
// [ADR-0105 D2] The `group` wall's predicate — the field the
87+
// share-link route used to drop before this call could see it.
88+
accessibleOrgIds: ctx.accessible_org_ids,
89+
objectHasOrgIdField: true,
90+
tenancyDisabled: false,
91+
posturePermitsCrossTenant: false,
92+
isPlatformAdmin: false,
93+
});
94+
if (layer0) rows = rows.filter((r) => matches(r, layer0));
95+
}
96+
return rows.filter((r) => matches(r, opts?.where ?? {}));
97+
},
98+
async insert(object: string, row: any) {
99+
(tables[object] ??= []).push(row);
100+
return row;
101+
},
102+
async update(object: string, data: any, options?: any) {
103+
const dispatch = assertEngineUpdateDispatch(data, options);
104+
const rows = tables[object] ?? [];
105+
if (dispatch.kind === 'by-id') {
106+
const i = rows.findIndex((r) => r.id === dispatch.id);
107+
if (i >= 0) rows[i] = { ...rows[i], ...data };
108+
return data;
109+
}
110+
const matched = rows.filter((r) => matches(r, options?.where ?? {}));
111+
for (const r of matched) Object.assign(r, data);
112+
return matched.length;
113+
},
114+
async delete(object: string, options?: any) {
115+
const dispatch = assertEngineDeleteDispatch(options);
116+
const rows = tables[object] ?? [];
117+
if (dispatch.kind === 'by-id') {
118+
const before = rows.length;
119+
tables[object] = rows.filter((r) => r.id !== dispatch.id);
120+
return tables[object].length < before;
121+
}
122+
const matched = rows.filter((r) => matches(r, options?.where ?? {}));
123+
tables[object] = rows.filter((r) => !matched.includes(r));
124+
return matched.length;
125+
},
126+
getSchema(object: string) {
127+
return object === OBJECT
128+
? {
129+
name: OBJECT,
130+
publicSharing: {
131+
enabled: true,
132+
allowedAudiences: ['link_only'],
133+
allowedPermissions: ['view'],
134+
},
135+
}
136+
: { name: object };
137+
},
138+
};
139+
}
140+
141+
class MockHttp {
142+
routes = new Map<string, any>();
143+
private add(method: string, path: string, handler: any) { this.routes.set(`${method} ${path}`, handler); }
144+
get(path: string, h: any) { this.add('GET', path, h); return this as any; }
145+
post(path: string, h: any) { this.add('POST', path, h); return this as any; }
146+
put(path: string, h: any) { this.add('PUT', path, h); return this as any; }
147+
delete(path: string, h: any) { this.add('DELETE', path, h); return this as any; }
148+
patch(path: string, h: any) { this.add('PATCH', path, h); return this as any; }
149+
use() { return this as any; }
150+
listen() { return Promise.resolve(); }
151+
close() { return Promise.resolve(); }
152+
getInstance() { return null; }
153+
}
154+
155+
interface MintOptions {
156+
/** The tenancy posture in force for this deployment. */
157+
posture: TenancyPosture;
158+
/** Organizations the caller holds a `sys_member` row in. */
159+
memberOf: string[];
160+
/** The record's owning organization. */
161+
recordOrg?: string;
162+
}
163+
164+
/**
165+
* Boot the real `SharingServicePlugin` and POST `/api/v1/share-links` for
166+
* `crm_account/acc_1` as a signed-in member — the exact call a user makes from
167+
* the record page's "share" button.
168+
*/
169+
async function groupPostureMint(opts: MintOptions): Promise<{ status: number; body: any }> {
170+
const userId = 'u_sharer';
171+
const activeOrg = opts.memberOf[0];
172+
const tables: Record<string, any[]> = {
173+
sys_user: [{ id: userId, email: 'sharer@example.com' }],
174+
sys_member: opts.memberOf.map((org, i) => ({
175+
id: `mem_${i}`,
176+
user_id: userId,
177+
organization_id: org,
178+
role: 'member',
179+
})),
180+
sys_user_position: [],
181+
sys_user_permission_set: [],
182+
sys_permission_set: [],
183+
[OBJECT]: [{ id: RECORD, name: 'Acme', organization_id: opts.recordOrg ?? ORG_A }],
184+
sys_share_link: [],
185+
};
186+
187+
const engine = makeEngine(tables, opts.posture);
188+
const http = new MockHttp();
189+
const hooks: Record<string, Array<() => Promise<void> | void>> = {};
190+
const ctx: any = {
191+
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
192+
hook: (event: string, handler: () => Promise<void> | void) => { (hooks[event] ??= []).push(handler); },
193+
getService: (name: string) => {
194+
if (name === 'objectql') return engine;
195+
if (name === 'http-server') return http;
196+
if (name === 'auth') {
197+
return {
198+
api: {
199+
getSession: async () => ({
200+
user: { id: userId, email: 'sharer@example.com' },
201+
session: { userId, activeOrganizationId: activeOrg },
202+
}),
203+
},
204+
};
205+
}
206+
throw new Error(`service not registered: ${name}`);
207+
},
208+
registerService: vi.fn(),
209+
};
210+
211+
const plugin = new SharingServicePlugin({ enforce: false });
212+
await plugin.start(ctx);
213+
for (const handler of hooks['kernel:ready'] ?? []) await handler();
214+
215+
const handler = http.routes.get(`POST ${BASE}`);
216+
if (!handler) throw new Error('share-link create route was not mounted');
217+
const captured: { status: number; body: any } = { status: 200, body: undefined };
218+
const res: any = {
219+
json: (data: any) => { captured.body = data; },
220+
send: () => undefined,
221+
status: (code: number) => { captured.status = code; return res; },
222+
header: () => res,
223+
};
224+
await handler(
225+
{
226+
params: {},
227+
query: {},
228+
body: { object: OBJECT, recordId: RECORD },
229+
headers: { cookie: 'better-auth.session_token=t' },
230+
method: 'POST',
231+
path: BASE,
232+
},
233+
res,
234+
);
235+
return captured;
236+
}
237+
238+
describe('[#6206] share-link creation under the `group` tenancy posture', () => {
239+
it('mints a link for a record the caller can read (403 before the envelope was passed through whole)', async () => {
240+
const res = await groupPostureMint({ posture: 'group', memberOf: [ORG_A] });
241+
242+
expect(res.status).toBe(201);
243+
expect(res.body).toMatchObject({ success: true });
244+
expect(res.body.data).toMatchObject({ object_name: OBJECT, record_id: RECORD });
245+
expect(typeof res.body.data.token).toBe('string');
246+
});
247+
248+
it('still refuses a record OUTSIDE the caller org access set — the wall is live, not bypassed', async () => {
249+
// Same posture, same route, same code: the caller belongs to plant B and
250+
// the record belongs to plant A, so Layer 0's `$in` predicate excludes it
251+
// and the mint is refused. This is what separates "the envelope now
252+
// arrives" from "the check was disabled".
253+
const res = await groupPostureMint({ posture: 'group', memberOf: [ORG_B], recordOrg: ORG_A });
254+
255+
expect(res.status).toBe(403);
256+
expect(res.body).toMatchObject({ success: false, error: { code: 'FORBIDDEN' } });
257+
});
258+
259+
it('reaches records across EVERY organization the caller belongs to (MOAC union)', async () => {
260+
const res = await groupPostureMint({ posture: 'group', memberOf: [ORG_B, ORG_A], recordOrg: ORG_A });
261+
expect(res.status).toBe(201);
262+
});
263+
264+
it('`single` posture is unchanged — Layer 0 is inert there, before and after', async () => {
265+
const res = await groupPostureMint({ posture: 'single', memberOf: [ORG_A] });
266+
expect(res.status).toBe(201);
267+
});
268+
});

0 commit comments

Comments
 (0)