From 2e2a93fc4c546b4fe02daf81ac943f0e3d9507b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:22:54 +0000 Subject: [PATCH] fix(plugin-sharing): share-link enforcement takes the whole authz envelope (#6206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv --- .changeset/share-link-route-full-envelope.md | 27 ++ .../src/share-link-tenant-wall.test.ts | 268 +++++++++++++++ .../share-link-enforcement-context.test.ts | 305 ++++++++++++++++++ .../plugin-sharing/src/share-link-routes.ts | 35 +- .../plugin-sharing/src/share-link-service.ts | 32 +- .../plugin-sharing/src/sharing-plugin.ts | 44 ++- 6 files changed, 689 insertions(+), 22 deletions(-) create mode 100644 .changeset/share-link-route-full-envelope.md create mode 100644 packages/plugins/plugin-security/src/share-link-tenant-wall.test.ts create mode 100644 packages/plugins/plugin-sharing/src/share-link-enforcement-context.test.ts diff --git a/.changeset/share-link-route-full-envelope.md b/.changeset/share-link-route-full-envelope.md new file mode 100644 index 0000000000..9c3cbf809f --- /dev/null +++ b/.changeset/share-link-route-full-envelope.md @@ -0,0 +1,27 @@ +--- +"@objectstack/plugin-sharing": patch +--- + +fix(plugin-sharing): share-link 路由把完整授权信封交给 enforcement,修复 `group` 姿态下建链恒 403(#6206,裁决 A 案的消费半边) + +`SharingServicePlugin` 的 share-link 路由此前在 `resolveAuthzContext` 之后重新 +拼一个四字段对象(`userId` / `tenantId` / `positions` / `permissions`),而这个 +对象被原样当作 enforcement context 喂进 `engine.find` —— 即 [Finding-2] +「只能为你自己看得见的记录建链接」那道可见性校验。被丢在半路的是 +`accessible_org_ids`、`org_user_ids`、`systemPermissions`、`posture`、 +`tabPermissions`。 + +实害(已复现,非仅代码读出):`group` 租户姿态下 `accessible_org_ids` 就是 +Layer 0 那堵墙(ADR-0105 D2),集合缺席即判否(fail closed)。于是可见性校验 +查不到任何行,建链接对**调用方本来读得到的记录**返回 +`403 FORBIDDEN: Not permitted to share /` —— 一个已发布姿态上, +已发布功能完全不可用。`single` 姿态(默认)不读该字段,行为不变。 + +改法按维护者 2026-08-07 的 A 案裁决(契约半边 #6430 / PR #6511 已落):信封 +**整个**透传(`{ ...authz, isSystem: false }`),不再逐字段挑选 —— 逐字段挑选正是 +这条缝出问题的方式,也是下一个新增授权维度会再次漏掉的地方。`posture` 随上下文 +流动、不在 enforcement 处重推(ADR-0095 D2)。窄类型 `ShareLinkExecutionContext` +保留,但只服务路由自己的 401 判定(认证与否),不再出现在任何裁决路径上。 + +`ShareLinkService.createLink` / `revokeLink` / `listLinks` 与 `canManageShares` +探针的参数类型随之收成完整 `ExecutionContext`,与 #6511 落地的契约一致。 diff --git a/packages/plugins/plugin-security/src/share-link-tenant-wall.test.ts b/packages/plugins/plugin-security/src/share-link-tenant-wall.test.ts new file mode 100644 index 0000000000..44002e747b --- /dev/null +++ b/packages/plugins/plugin-security/src/share-link-tenant-wall.test.ts @@ -0,0 +1,268 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6206 / #6430 ruling A] The `group`-posture repro: minting a share link for + * a record the caller can read. + * + * ## Why this file lives in plugin-SECURITY + * + * The defect is a seam in `@objectstack/plugin-sharing` (its share-link routes + * rebuilt a four-field subset of the `resolveAuthzContext` envelope and fed it + * to `engine.find` as the [Finding-2] visibility check's context), but the + * VERDICT that made it a 403 is computed here: `computeTenantLayer0Filter` + * reads `ExecutionContext.accessible_org_ids` and, under the `group` posture, + * an absent/empty set denies (ADR-0105 D2, fail closed). Proving the bug + * therefore needs both packages in one process, and this is the one that owns + * the wall — plugin-security already depends on plugin-sharing for the same + * reason (`controlled-by-parent-sharing.test.ts`, + * `vama-write-path-convergence.test.ts`), never the other way round. + * + * ## What is real here and what is a double + * + * REAL: the plugin's own route wiring and context assembly (the plugin is + * booted, so the closure under test is the production one), the share-link + * service, and the tenant wall — `computeTenantLayer0Filter` is called with the + * context the route actually produced, exactly as `security-plugin.ts` calls it + * on a read. + * + * DOUBLE: storage. The engine below is an in-memory table set that applies the + * wall the same way the security middleware does — AND-composed first, on a + * non-system context — so `RLS_DENY_FILTER` denies by being an unmatchable + * predicate rather than by a special case, which is how it denies in + * production. + * + * ## Before/after, recorded + * + * With the four-field assembly restored in plugin-sharing, `groupPostureMint` + * answers 403 (`FORBIDDEN: Not permitted to share crm_account/acc_1`) — the + * card's repro — while the `single`-posture case stays 201. After the fix the + * `group` case is 201 and the `single` case is unchanged. The third case is the + * one that keeps the fix honest: a caller with no membership in the record's + * organization must STILL be refused, because the envelope was widened, not the + * authority. + */ + +import { describe, it, expect, vi } from 'vitest'; +// The producers' OWN dispatch predicates for the double's write verbs, from +// `@objectstack/metadata-core` (where they live since #5619) — this package +// does not depend on `@objectstack/objectql`, and taking that edge to reach the +// re-export would be a cycle turbo refuses. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import type { TenancyPosture } from '@objectstack/spec/security'; +import { SharingServicePlugin } from '@objectstack/plugin-sharing'; +import { computeTenantLayer0Filter } from './tenant-layer.js'; + +const BASE = '/api/v1/share-links'; +const OBJECT = 'crm_account'; +const RECORD = 'acc_1'; +const ORG_A = 'org_plant_a'; +const ORG_B = 'org_plant_b'; + +/** Objects that carry `organization_id` — the wall's "is this a tenant object?" input. */ +const TENANT_OBJECTS = new Set([OBJECT]); + +function matches(row: any, where: Record): boolean { + return Object.entries(where).every(([k, v]) => { + if (v && typeof v === 'object' && '$in' in v) return (v as any).$in.includes(row[k]); + return row[k] === v; + }); +} + +/** + * An engine that enforces Layer 0 exactly as the security middleware does: the + * REAL `computeTenantLayer0Filter`, fed the caller's context, AND-composed onto + * the query's own predicate. A system context bypasses it, as it does in + * production. + */ +function makeEngine(tables: Record, posture: TenancyPosture) { + return { + async find(object: string, opts: any) { + const ctx = opts?.context ?? {}; + let rows = tables[object] ?? []; + if (!ctx.isSystem && TENANT_OBJECTS.has(object)) { + const layer0 = computeTenantLayer0Filter({ + tenancyPosture: posture, + organizationId: ctx.tenantId, + // [ADR-0105 D2] The `group` wall's predicate — the field the + // share-link route used to drop before this call could see it. + accessibleOrgIds: ctx.accessible_org_ids, + objectHasOrgIdField: true, + tenancyDisabled: false, + posturePermitsCrossTenant: false, + isPlatformAdmin: false, + }); + if (layer0) rows = rows.filter((r) => matches(r, layer0)); + } + return rows.filter((r) => matches(r, opts?.where ?? {})); + }, + async insert(object: string, row: any) { + (tables[object] ??= []).push(row); + return row; + }, + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const rows = tables[object] ?? []; + if (dispatch.kind === 'by-id') { + const i = rows.findIndex((r) => r.id === dispatch.id); + if (i >= 0) rows[i] = { ...rows[i], ...data }; + return data; + } + const matched = rows.filter((r) => matches(r, options?.where ?? {})); + for (const r of matched) Object.assign(r, data); + return matched.length; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const rows = tables[object] ?? []; + if (dispatch.kind === 'by-id') { + const before = rows.length; + tables[object] = rows.filter((r) => r.id !== dispatch.id); + return tables[object].length < before; + } + const matched = rows.filter((r) => matches(r, options?.where ?? {})); + tables[object] = rows.filter((r) => !matched.includes(r)); + return matched.length; + }, + getSchema(object: string) { + return object === OBJECT + ? { + name: OBJECT, + publicSharing: { + enabled: true, + allowedAudiences: ['link_only'], + allowedPermissions: ['view'], + }, + } + : { name: object }; + }, + }; +} + +class MockHttp { + routes = new Map(); + private add(method: string, path: string, handler: any) { this.routes.set(`${method} ${path}`, handler); } + get(path: string, h: any) { this.add('GET', path, h); return this as any; } + post(path: string, h: any) { this.add('POST', path, h); return this as any; } + put(path: string, h: any) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: any) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: any) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +interface MintOptions { + /** The tenancy posture in force for this deployment. */ + posture: TenancyPosture; + /** Organizations the caller holds a `sys_member` row in. */ + memberOf: string[]; + /** The record's owning organization. */ + recordOrg?: string; +} + +/** + * Boot the real `SharingServicePlugin` and POST `/api/v1/share-links` for + * `crm_account/acc_1` as a signed-in member — the exact call a user makes from + * the record page's "share" button. + */ +async function groupPostureMint(opts: MintOptions): Promise<{ status: number; body: any }> { + const userId = 'u_sharer'; + const activeOrg = opts.memberOf[0]; + const tables: Record = { + sys_user: [{ id: userId, email: 'sharer@example.com' }], + sys_member: opts.memberOf.map((org, i) => ({ + id: `mem_${i}`, + user_id: userId, + organization_id: org, + role: 'member', + })), + sys_user_position: [], + sys_user_permission_set: [], + sys_permission_set: [], + [OBJECT]: [{ id: RECORD, name: 'Acme', organization_id: opts.recordOrg ?? ORG_A }], + sys_share_link: [], + }; + + const engine = makeEngine(tables, opts.posture); + const http = new MockHttp(); + const hooks: Record Promise | void>> = {}; + const ctx: any = { + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + hook: (event: string, handler: () => Promise | void) => { (hooks[event] ??= []).push(handler); }, + getService: (name: string) => { + if (name === 'objectql') return engine; + if (name === 'http-server') return http; + if (name === 'auth') { + return { + api: { + getSession: async () => ({ + user: { id: userId, email: 'sharer@example.com' }, + session: { userId, activeOrganizationId: activeOrg }, + }), + }, + }; + } + throw new Error(`service not registered: ${name}`); + }, + registerService: vi.fn(), + }; + + const plugin = new SharingServicePlugin({ enforce: false }); + await plugin.start(ctx); + for (const handler of hooks['kernel:ready'] ?? []) await handler(); + + const handler = http.routes.get(`POST ${BASE}`); + if (!handler) throw new Error('share-link create route was not mounted'); + const captured: { status: number; body: any } = { status: 200, body: undefined }; + const res: any = { + json: (data: any) => { captured.body = data; }, + send: () => undefined, + status: (code: number) => { captured.status = code; return res; }, + header: () => res, + }; + await handler( + { + params: {}, + query: {}, + body: { object: OBJECT, recordId: RECORD }, + headers: { cookie: 'better-auth.session_token=t' }, + method: 'POST', + path: BASE, + }, + res, + ); + return captured; +} + +describe('[#6206] share-link creation under the `group` tenancy posture', () => { + it('mints a link for a record the caller can read (403 before the envelope was passed through whole)', async () => { + const res = await groupPostureMint({ posture: 'group', memberOf: [ORG_A] }); + + expect(res.status).toBe(201); + expect(res.body).toMatchObject({ success: true }); + expect(res.body.data).toMatchObject({ object_name: OBJECT, record_id: RECORD }); + expect(typeof res.body.data.token).toBe('string'); + }); + + it('still refuses a record OUTSIDE the caller org access set — the wall is live, not bypassed', async () => { + // Same posture, same route, same code: the caller belongs to plant B and + // the record belongs to plant A, so Layer 0's `$in` predicate excludes it + // and the mint is refused. This is what separates "the envelope now + // arrives" from "the check was disabled". + const res = await groupPostureMint({ posture: 'group', memberOf: [ORG_B], recordOrg: ORG_A }); + + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ success: false, error: { code: 'FORBIDDEN' } }); + }); + + it('reaches records across EVERY organization the caller belongs to (MOAC union)', async () => { + const res = await groupPostureMint({ posture: 'group', memberOf: [ORG_B, ORG_A], recordOrg: ORG_A }); + expect(res.status).toBe(201); + }); + + it('`single` posture is unchanged — Layer 0 is inert there, before and after', async () => { + const res = await groupPostureMint({ posture: 'single', memberOf: [ORG_A] }); + expect(res.status).toBe(201); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/share-link-enforcement-context.test.ts b/packages/plugins/plugin-sharing/src/share-link-enforcement-context.test.ts new file mode 100644 index 0000000000..89d4e461b4 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/share-link-enforcement-context.test.ts @@ -0,0 +1,305 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6206 / #6430 ruling A] What the share-link routes hand to ENFORCEMENT. + * + * ## The defect + * + * `SharingServicePlugin` resolved the caller through `resolveAuthzContext` — + * the one shared authorization resolver — and then rebuilt a FOUR-FIELD object + * out of the result (`userId` / `tenantId` / `positions` / `permissions`). + * That object was not a route-local identity: it went straight into + * `ShareLinkService.createLink`, which reads the target record under it + * ([Finding-2] "you may only link-share a record you can see"). So + * `accessible_org_ids`, `org_user_ids`, `systemPermissions`, `posture` and + * `tabPermissions` were dropped ON THE WAY INTO the data engine. Under the + * `group` tenancy posture `accessible_org_ids` IS the Layer 0 wall (ADR-0105 + * D2) and an absent set denies, so the visibility read found nothing and link + * creation answered 403 for records the caller reads fine elsewhere. The + * behavioural half of that proof — the wall itself, before and after — lives in + * `plugin-security`'s `share-link-tenant-wall.test.ts`, which is the package + * that owns `computeTenantLayer0Filter`; this file pins the seam that feeds it. + * + * ## Why the pin is written against the SEAM rather than a key list + * + * A hand-written list of "the fields enforcement needs" is the same artefact + * that broke here: it was correct when written and silently wrong the moment + * the envelope grew a dimension. So the assertion compares the enforcement + * context against a context the REAL resolver produced for the same principal + * (`bootRequestContext`, the #5859 seam kit) — every key the resolver emits + * must be present. A new authorization dimension is then covered on the day it + * is added, by both sides at once, and re-trimming this seam fails by NAMING + * the keys it dropped. + * + * The five keys the card names are additionally asserted by name, because a + * reader of this file should be able to see the issue's own vocabulary in it. + */ + +import { describe, it, expect, vi } from 'vitest'; +// The producers' OWN dispatch predicates: a double that opens its write verbs +// with these cannot accept a call the real engine refuses +// (`check:engine-double-contract`). +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import type { + IHttpServer, + IHttpRequest, + IHttpResponse, + RouteHandler, +} from '@objectstack/spec/contracts'; +import { SharingServicePlugin } from './sharing-plugin.js'; +import { bootRequestContext } from './exec-context-seam.testkit.js'; + +const BASE = '/api/v1/share-links'; +const USER = 'u_sharer'; +const EMAIL = 'sharer@example.com'; +const ORG = 'org_plant_a'; +const OBJECT = 'crm_account'; +const RECORD = 'acc_1'; + +// ── Test doubles ───────────────────────────────────────────────────────────── + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +/** `where` matching good enough for both the identity reads and the record read. */ +function matches(row: any, where: Record): boolean { + return Object.entries(where).every(([k, v]) => { + if (v && typeof v === 'object' && '$in' in v) return (v as any).$in.includes(row[k]); + return row[k] === v; + }); +} + +interface FindCall { object: string; context: any } + +/** + * An engine that answers the identity tables `resolveAuthzContext` reads AND + * the business object under test, recording the context of every read so the + * assertions can look at what enforcement was actually handed. + */ +function makeEngine(tables: Record) { + const finds: FindCall[] = []; + return { + finds, + async find(object: string, opts: any) { + finds.push({ object, context: opts?.context }); + return (tables[object] ?? []).filter((r) => matches(r, opts?.where ?? {})); + }, + async insert(object: string, row: any) { + (tables[object] ??= []).push(row); + return row; + }, + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const rows = tables[object] ?? []; + if (dispatch.kind === 'by-id') { + const i = rows.findIndex((r) => r.id === dispatch.id); + if (i >= 0) rows[i] = { ...rows[i], ...data }; + return data; + } + const matched = rows.filter((r) => matches(r, options?.where ?? {})); + for (const r of matched) Object.assign(r, data); + return matched.length; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const rows = tables[object] ?? []; + if (dispatch.kind === 'by-id') { + const before = rows.length; + tables[object] = rows.filter((r) => r.id !== dispatch.id); + return tables[object].length < before; + } + const matched = rows.filter((r) => matches(r, options?.where ?? {})); + tables[object] = rows.filter((r) => !matched.includes(r)); + return matched.length; + }, + getSchema(object: string) { + return object === OBJECT + ? { + name: OBJECT, + publicSharing: { + enabled: true, + allowedAudiences: ['link_only'], + allowedPermissions: ['view'], + }, + } + : { name: object }; + }, + }; +} + +/** + * Boot the REAL plugin: the assembly under test is a closure inside its + * `kernel:ready` handler, so nothing short of starting the plugin exercises the + * production wiring. `enforce: false` keeps this to the share-link surface — + * the sharing middleware and the rule subsystem are a different card's code and + * would only add noise here (the share-link service is registered in that + * posture too, by design). + */ +async function bootPlugin(opts: { session: () => any; tables: Record }) { + const engine = makeEngine(opts.tables); + const http = new MockHttp(); + const hooks: Record Promise | void>> = {}; + const ctx: any = { + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + hook: (event: string, handler: () => Promise | void) => { + (hooks[event] ??= []).push(handler); + }, + getService: (name: string) => { + if (name === 'objectql') return engine; + if (name === 'http-server') return http; + if (name === 'auth') return { api: { getSession: async () => opts.session() } }; + throw new Error(`service not registered: ${name}`); + }, + registerService: vi.fn(), + }; + + const plugin = new SharingServicePlugin({ enforce: false }); + await plugin.start(ctx); + for (const handler of hooks['kernel:ready'] ?? []) await handler(); + + return { engine, http }; +} + +interface Captured { status: number; body: any } + +async function drive( + http: MockHttp, + key: string, + opts: { params?: Record; body?: any; query?: any; headers?: any } = {}, +): Promise { + const handler = http.routes.get(key); + if (!handler) throw new Error(`no handler for ${key}`); + const captured: Captured = { status: 200, body: undefined }; + const res: IHttpResponse = { + json: vi.fn((data: any) => { captured.body = data; }) as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { captured.status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + const req: IHttpRequest = { + params: opts.params ?? {}, + query: opts.query ?? {}, + body: opts.body, + headers: opts.headers ?? { cookie: 'better-auth.session_token=t' }, + method: 'POST', + path: BASE, + }; + await handler(req, res); + return captured; +} + +/** The identity + business rows a real deployment holds for this principal. */ +function fixtureTables(): Record { + return { + sys_user: [{ id: USER, email: EMAIL }], + sys_member: [{ id: 'mem_1', user_id: USER, organization_id: ORG, role: 'member' }], + sys_user_position: [], + sys_user_permission_set: [], + sys_permission_set: [], + [OBJECT]: [{ id: RECORD, name: 'Acme', organization_id: ORG }], + sys_share_link: [], + }; +} + +/** A better-auth session exactly as `AuthManager` hands it to the resolver. */ +const signedIn = () => ({ + user: { id: USER, email: EMAIL }, + session: { userId: USER, activeOrganizationId: ORG }, +}); + +// ── The pin ────────────────────────────────────────────────────────────────── + +describe('[#6206] share-link routes hand ENFORCEMENT the whole authz envelope', () => { + it('the visibility read of createLink runs on every key the resolver produced', async () => { + const tables = fixtureTables(); + const { engine, http } = await bootPlugin({ session: signedIn, tables }); + + const res = await drive(http, `POST ${BASE}`, { body: { object: OBJECT, recordId: RECORD } }); + expect(res.status).toBe(201); + + // The [Finding-2] visibility read — the call this card is about. + const probe = engine.finds.find((f) => f.object === OBJECT); + expect(probe, 'createLink must re-read the record under the caller context').toBeDefined(); + const enforcementCtx: any = probe!.context; + + // The reference envelope: the SAME resolver, the same principal, produced + // through the #5859 seam kit rather than hand-written here. + const seam: any = await bootRequestContext({ + userId: USER, + email: EMAIL, + activeOrganizationId: ORG, + }); + const dropped = Object.keys(seam).filter((k) => !(k in enforcementCtx)); + expect(dropped, 'keys the route dropped on the way into enforcement').toEqual([]); + + // The four wall/authorization inputs the card names by hand, with the + // values a `group`-posture deployment depends on. (`tabPermissions` is the + // fifth; it is emitted only when the principal holds tab overrides, so it + // is covered by the seam comparison above rather than by a value here.) + expect(enforcementCtx.accessible_org_ids).toEqual([ORG]); + expect(enforcementCtx.org_user_ids).toContain(USER); + expect(Array.isArray(enforcementCtx.systemPermissions)).toBe(true); + // [ADR-0095 D2] Resolved ONCE, upstream, and carried — this assertion is + // that it arrives, not that anything here re-derives it. + expect(enforcementCtx.posture).toBe('MEMBER'); + + // The four fields the old assembly did carry are unchanged. + expect(enforcementCtx.userId).toBe(USER); + expect(enforcementCtx.tenantId).toBe(ORG); + expect(enforcementCtx.positions).toEqual(seam.positions); + expect(enforcementCtx.permissions).toEqual(seam.permissions); + + // [ADR-0118 D2] Absence is never system — the route says so explicitly, and + // must never say the opposite (that would bypass the read entirely). + expect(enforcementCtx.isSystem).toBe(false); + }); + + it('listLinks reads under the same whole envelope', async () => { + const tables = fixtureTables(); + const { engine, http } = await bootPlugin({ session: signedIn, tables }); + + const res = await drive(http, `GET ${BASE}`, { query: {} }); + expect(res.status).toBe(200); + + const listRead = engine.finds.filter((f) => f.object === 'sys_share_link').pop(); + expect(listRead).toBeDefined(); + expect(listRead!.context.accessible_org_ids).toEqual([ORG]); + expect(listRead!.context.posture).toBe('MEMBER'); + }); + + it('an unresolvable request is still anonymous → 401, and nothing is enforced', async () => { + const tables = fixtureTables(); + const { engine, http } = await bootPlugin({ session: () => undefined, tables }); + + const res = await drive(http, `POST ${BASE}`, { body: { object: OBJECT, recordId: RECORD } }); + expect(res.status).toBe(401); + expect(res.body).toMatchObject({ success: false, error: { code: 'UNAUTHENTICATED' } }); + // The 401 is decided before any enforcement runs: no read of the record. + expect(engine.finds.some((f) => f.object === OBJECT)).toBe(false); + }); + + it('a record the caller cannot see is still refused (the [Finding-2] guarantee)', async () => { + // Widening the envelope must not widen WHO may mint a link: an engine that + // reports the row as invisible still yields 403. + const tables = fixtureTables(); + tables[OBJECT] = []; + const { http } = await bootPlugin({ session: signedIn, tables }); + + const res = await drive(http, `POST ${BASE}`, { body: { object: OBJECT, recordId: RECORD } }); + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ success: false, error: { code: 'FORBIDDEN' } }); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/share-link-routes.ts b/packages/plugins/plugin-sharing/src/share-link-routes.ts index e30a5bc7f5..28ce112f42 100644 --- a/packages/plugins/plugin-sharing/src/share-link-routes.ts +++ b/packages/plugins/plugin-sharing/src/share-link-routes.ts @@ -34,6 +34,7 @@ import type { IHttpServer, IHttpRequest, RouteHandler } from '@objectstack/spec/ // The declared envelope is written in ONE place for the whole platform (#3973). import { sendOk, sendError } from '@objectstack/types'; import type { ShareLinkExecutionContext } from '@objectstack/spec/contracts'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { ShareLinkService } from './share-link-service.js'; import type { SharingEngine } from './sharing-service.js'; @@ -50,14 +51,38 @@ export interface ShareLinkRoutesOptions { * an anonymous context (the authenticated routes then 401). The old default * trusted `x-user-id` / `x-tenant-id`, which let a client forge attribution * and enumerate/revoke other users' links. + * + * [#6206 / #6430] It returns the FULL {@link ExecutionContext} — the whole + * `resolveAuthzContext` envelope — because this module forwards it unchanged + * into `createLink` / `listLinks` / `revokeLink`, every one of which + * ADJUDICATES access. A resolver that rebuilds a subset here silently changes + * those verdicts: `accessible_org_ids` is the `group`-posture Layer 0 wall + * (ADR-0105 D2) and denies when absent. The routes' OWN decision — is this + * request authenticated at all? — is the only thing they read off it + * themselves, via {@link isAuthenticated}. */ - contextFromRequest?: (req: IHttpRequest) => ShareLinkExecutionContext | Promise; + contextFromRequest?: (req: IHttpRequest) => ExecutionContext | Promise; } // [Finding-2] Secure default: anonymous (no identity read from headers). A // deployment that wants authenticated share-link management must wire a // verified `contextFromRequest` (the plugin does). -const defaultContext = (_req: IHttpRequest): ShareLinkExecutionContext => ({}); +const defaultContext = (_req: IHttpRequest): ExecutionContext => ({}); + +/** + * [#6206] The routes' own 401 gate — authenticated vs anonymous, and nothing + * more. + * + * Typed to {@link ShareLinkExecutionContext} deliberately: that is the shape + * the contract retains for exactly this decision, and narrowing HERE (at the + * read) rather than at the resolver (at the production site) is the whole point + * of the ruling. The gate reads no authorization dimension, so it needs no + * authorization envelope — while the object the routes hand on to the service + * stays the complete one. + */ +function isAuthenticated(ctx: ShareLinkExecutionContext): boolean { + return Boolean(ctx.userId); +} /** * ## Why `data` carries the payload bare on this module's five routes @@ -117,7 +142,7 @@ export function registerShareLinkRoutes( try { const ctx = await ctxOf(req); // [Finding-2] Managing links requires a verified principal. - if (!ctx.userId) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to create share links'); + if (!isAuthenticated(ctx)) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to create share links'); const body: any = req.body ?? {}; if (!body.object || !body.recordId) { return sendError(res, 400, 'VALIDATION_FAILED', 'object and recordId are required'); @@ -150,7 +175,7 @@ export function registerShareLinkRoutes( http.get(base, (async (req, res) => { try { const ctx = await ctxOf(req); - if (!ctx.userId) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to list share links'); + if (!isAuthenticated(ctx)) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to list share links'); const q = req.query ?? {}; const links = await service.listLinks( { @@ -173,7 +198,7 @@ export function registerShareLinkRoutes( http.delete(`${base}/:idOrToken`, (async (req, res) => { try { const ctx = await ctxOf(req); - if (!ctx.userId) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to revoke share links'); + if (!isAuthenticated(ctx)) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to revoke share links'); await service.revokeLink(req.params.idOrToken, ctx); // `{ ok: true }` moves from BEING the body to being its `data`. It was a // second word for `success` at the top level (#3689 retired that from diff --git a/packages/plugins/plugin-sharing/src/share-link-service.ts b/packages/plugins/plugin-sharing/src/share-link-service.ts index 603304915c..34a81a04ff 100644 --- a/packages/plugins/plugin-sharing/src/share-link-service.ts +++ b/packages/plugins/plugin-sharing/src/share-link-service.ts @@ -6,10 +6,18 @@ import type { CreateShareLinkInput, ListShareLinksFilter, ResolveShareLinkResult, - ShareLinkExecutionContext, ShareLinkPermission, ShareLinkAudience, } from '@objectstack/spec/contracts'; +/** + * [#6206 / #6430 — maintainer ruling A] Every method here that adjudicates + * access takes the FULL envelope. The route-local `ShareLinkExecutionContext` + * is the HTTP layer's 401 vocabulary and is deliberately not named in this + * file: the contexts this file receives are forwarded into `engine.find`, where + * `accessible_org_ids` (ADR-0105 D2), `posture` (ADR-0095 D2), `org_user_ids`, + * `systemPermissions` and `tabPermissions` are all read. + */ +import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { SharingEngine } from './sharing-service.js'; import { deleteRowsForDeletedRecords, @@ -194,11 +202,15 @@ export interface ShareLinkServiceOptions { * revoke a link someone else minted on their record — not just the link's * creator. Absent → only the creator (and system) may revoke, the pre-D8 * behaviour, so a deployment without the sharing service degrades safely. + * + * [#6206] An ENFORCEMENT probe — it decides a 403, and resolves ownership / + * hierarchy scope under the context it is given — so it receives the caller's + * COMPLETE envelope, exactly like the visibility read in `createLink`. */ canManageShares?: ( object: string, recordId: string, - context: ShareLinkExecutionContext, + context: ExecutionContext, ) => Promise; /** [#5190] Optional logger for the record-delete cascade / orphan sweep. */ logger?: { info?: Function; warn?: Function; error?: Function; debug?: Function }; @@ -221,7 +233,7 @@ export class ShareLinkService implements IShareLinkService { private readonly canManageShares?: ( object: string, recordId: string, - context: ShareLinkExecutionContext, + context: ExecutionContext, ) => Promise; private readonly logger?: ShareLinkServiceOptions['logger']; @@ -236,7 +248,7 @@ export class ShareLinkService implements IShareLinkService { async createLink( input: CreateShareLinkInput, - context: ShareLinkExecutionContext, + context: ExecutionContext, ): Promise { if (!input.object) throw makeError(400, 'VALIDATION_FAILED', 'object is required'); if (!input.recordId) throw makeError(400, 'VALIDATION_FAILED', 'recordId is required'); @@ -285,6 +297,14 @@ export class ShareLinkService implements IShareLinkService { // record you can access; a client can no longer share arbitrary rows of a // publicSharing-enabled object it cannot see. Internal (isSystem) callers // read under the system context as before. + // + // [#6206] `context` is passed through UNCHANGED — it is the caller's whole + // resolved envelope and every dimension of it is an input to this read: + // Layer 0 reads `accessible_org_ids` under the `group` posture (ADR-0105 + // D2, where an absent set denies), Layer 1 reads positions / permissions / + // `org_user_ids`, and `posture` travels with the context rather than being + // re-derived here (ADR-0095 D2). Rebuilding a subset at this seam is what + // made this check answer 403 for every `group`-posture caller. const exists = await this.engine.find(input.object, { where: { id: input.recordId }, fields: ['id'], @@ -329,7 +349,7 @@ export class ShareLinkService implements IShareLinkService { return row; } - async revokeLink(idOrToken: string, context: ShareLinkExecutionContext): Promise { + async revokeLink(idOrToken: string, context: ExecutionContext): Promise { if (!idOrToken) throw makeError(400, 'VALIDATION_FAILED', 'id or token is required'); const filter = idOrToken.startsWith('shl_') ? { id: idOrToken } : { token: idOrToken }; const rows = await this.engine.find('sys_share_link', { @@ -369,7 +389,7 @@ export class ShareLinkService implements IShareLinkService { async listLinks( filter: ListShareLinksFilter, - context: ShareLinkExecutionContext, + context: ExecutionContext, ): Promise { const where: Record = {}; if (filter.object) where.object_name = filter.object; diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index bde25cda5e..e970198434 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -3,7 +3,11 @@ import type { Plugin, PluginContext } from '@objectstack/core'; import { resolveAuthzContext } from '@objectstack/core'; import type { EngineMiddleware, OperationContext } from '@objectstack/objectql'; -import type { IHttpServer, IHttpRequest, ShareLinkExecutionContext } from '@objectstack/spec/contracts'; +import type { IHttpServer, IHttpRequest } from '@objectstack/spec/contracts'; +// [#6206] The share-link routes' context is the FULL authorization envelope — +// it feeds enforcement (`engine.find`), so it is an `ExecutionContext`, never +// the route-local `ShareLinkExecutionContext`. +import type { ExecutionContext } from '@objectstack/spec/kernel'; import { SysRecordShare, SysSharingRule, SysShareLink } from './objects/index.js'; import { SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity'; import { SharingService, type SharingEngine, type SharingTenancyProbe } from './sharing-service.js'; @@ -609,11 +613,30 @@ export class SharingServicePlugin implements Plugin { if (http) { // [Finding-2] Derive the caller from the platform's VERIFIED // resolution (session / API key / OAuth), never from spoofable - // `x-user-id` headers. `positions`/`permissions` flow through so the - // createLink record-access check evaluates real RLS. An - // unresolvable request → anonymous (the authed routes then 401). + // `x-user-id` headers. An unresolvable request → anonymous (the + // authed routes then 401). + // + // [#6206 / #6430 — maintainer ruling A, 2026-08-07] The envelope is + // handed on WHOLE. This assembly used to name four fields + // (`userId`/`tenantId`/`positions`/`permissions`) and the resulting + // object was passed straight into `engine.find` as the [Finding-2] + // visibility check's context — so `accessible_org_ids`, + // `org_user_ids`, `systemPermissions`, `posture` and + // `tabPermissions` never reached enforcement. Under the `group` + // tenancy posture `accessible_org_ids` IS the Layer 0 wall + // (ADR-0105 D2) and an absent set DENIES, so link creation answered + // a blanket 403 on a posture that ships. `posture` is likewise + // resolved once by the resolver and carried (ADR-0095 D2) — never + // re-derived at the enforcement site, which is exactly what a + // per-site subset forces the next layer to do. + // + // A spread, not a field list, on purpose: a field list is how this + // seam broke, and it breaks again the day `ResolvedAuthzContext` + // grows a dimension nobody remembers to add here. The route's own + // 401 decision reads `userId` off the same object (see + // `ShareLinkExecutionContext` in the contract for that boundary). const ql: any = engine; - const verifiedContextFromRequest = async (req: IHttpRequest): Promise => { + const verifiedContextFromRequest = async (req: IHttpRequest): Promise => { try { const headers = new Headers(); for (const [k, v] of Object.entries(req.headers ?? {})) { @@ -631,12 +654,11 @@ export class SharingServicePlugin implements Plugin { } }; const authz = await resolveAuthzContext({ ql, headers, getSession }); - return { - userId: authz.userId, - tenantId: authz.tenantId, - positions: authz.positions, - permissions: authz.permissions, - }; + // `isSystem: false` states what the absence of the flag already + // means (ADR-0118 D2: absence is never system) and matches what + // both sibling transports build — `rest-server.ts` and + // `runtime/src/security/resolve-execution-context.ts`. + return { ...authz, isSystem: false }; } catch { return {}; // anonymous → authed routes 401 }