fix(sharing): verified authz on share-link routes (#2851)

.changeset/share-link-routes-verified-authz.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/plugin-sharing': minor
+'@objectstack/spec': minor
+---
+
+**Security fix (#2851): the share-link HTTP routes no longer trust spoofable identity headers, and the service enforces ownership.**
+
+The raw-app share-link routes (`POST/GET/DELETE /api/v1/share-links`, registered by `SharingServicePlugin`) derived the caller from `x-user-id` / `x-tenant-id` request headers, and the service ignored the caller context on revoke. So a client could forge link attribution, enumerate another user's link tokens (`GET ?createdBy=<victim>` → tokens that resolve records under a system context, bypassing RLS), and revoke arbitrary users' links.
+
+Fixes:
+
+- **Verified identity.** `SharingServicePlugin` now derives the caller (and their positions/permissions) from the platform's verified resolution (`resolveAuthzContext` — session / API key / OAuth), never from headers. The route default is SECURE (anonymous). Create / list / revoke require a signed-in principal (401 otherwise); the public `/:token/resolve` route stays public (the token is the authorization) but keys its `audience: 'signed_in'` check off the verified session rather than a spoofable `x-user-id`.
+- **List scoping.** `GET /api/v1/share-links` is forced to the caller's own links — a client can no longer pass `?createdBy=<victim>` to enumerate others' tokens.
+- **Revoke ownership.** `revokeLink` now requires the caller to be the link's creator (system/internal callers bypass). Previously the caller context was ignored, so anyone could revoke any link (sharing DoS).
+- **Create access check.** `createLink` verifies the record is visible to the caller (read under the caller's own RLS) before minting a link — you can only share a record you can actually see. Internal (system) callers are unchanged.
+
+`ShareLinkExecutionContext` gains optional `positions` / `permissions` so the record-access check evaluates the real principal.
+
+Found by an adversarial security review of the request→ExecutionContext trust boundary (companion to the settings-routes fix, #2848).

packages/plugins/plugin-sharing/src/share-link-routes.ts

@@ -30,20 +30,23 @@ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
 
 export interface ShareLinkRoutesOptions {
   basePath?: string;
-  /** Read caller identity for authenticated routes. */
-  contextFromRequest?: (req: IHttpRequest) => ShareLinkExecutionContext;
+  /**
+   * Derive the VERIFIED caller identity for the authenticated routes
+   * (create / list / revoke). Production wiring (`SharingServicePlugin`) passes
+   * a resolver backed by `resolveAuthzContext` (session / API key / OAuth).
+   *
+   * [Finding-2] The default is SECURE: it trusts NO identity header and yields
+   * 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.
+   */
+  contextFromRequest?: (req: IHttpRequest) => ShareLinkExecutionContext | Promise<ShareLinkExecutionContext>;
 }
 
-const defaultContext = (req: IHttpRequest): ShareLinkExecutionContext => {
-  const header = (name: string): string | undefined => {
-    const v = req.headers?.[name];
-    return Array.isArray(v) ? v[0] : v;
-  };
-  return {
-    userId: header('x-user-id'),
-    tenantId: header('x-tenant-id'),
-  };
-};
+// [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 => ({});
 
 function sendError(res: IHttpResponse, status: number, code: string, message: string) {
   res.status(status).json({ error: { code, message } });
@@ -73,7 +76,9 @@ export function registerShareLinkRoutes(
   // ── CREATE ─────────────────────────────────────────────────────
   http.post(base, (async (req, res) => {
     try {
-      const ctx = ctxOf(req);
+      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');
       const body: any = req.body ?? {};
       if (!body.object || !body.recordId) {
         return sendError(res, 400, 'VALIDATION_FAILED', 'object and recordId are required');
@@ -105,13 +110,16 @@ export function registerShareLinkRoutes(
   // ── LIST ───────────────────────────────────────────────────────
   http.get(base, (async (req, res) => {
     try {
-      const ctx = ctxOf(req);
+      const ctx = await ctxOf(req);
+      if (!ctx.userId) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to list share links');
       const q = req.query ?? {};
       const link = await service.listLinks(
         {
           object: typeof q.object === 'string' ? q.object : undefined,
           recordId: typeof q.recordId === 'string' ? q.recordId : undefined,
-          createdBy: typeof q.createdBy === 'string' ? q.createdBy : undefined,
+          // [Finding-2] Force the caller's own id — a client can no longer pass
+          // `?createdBy=<victim>` to enumerate another user's link tokens.
+          createdBy: ctx.userId,
           includeRevoked: q.includeRevoked === 'true' || q.includeRevoked === '1',
         },
         ctx,
@@ -125,7 +133,8 @@ export function registerShareLinkRoutes(
   // ── REVOKE ─────────────────────────────────────────────────────
   http.delete(`${base}/:idOrToken`, (async (req, res) => {
     try {
-      const ctx = ctxOf(req);
+      const ctx = await ctxOf(req);
+      if (!ctx.userId) return sendError(res, 401, 'UNAUTHENTICATED', 'Sign in to revoke share links');
       await service.revokeLink(req.params.idOrToken, ctx);
       await res.status(200).json({ ok: true });
     } catch (err: any) {
@@ -140,10 +149,10 @@ export function registerShareLinkRoutes(
   http.get(`${base}/:token/resolve`, (async (req, res) => {
     try {
       const q = req.query ?? {};
-      const signedInUserId = (() => {
-        const v = req.headers?.['x-user-id'];
-        return Array.isArray(v) ? v[0] : v;
-      })();
+      // [Finding-2] The `audience: 'signed_in'` gate must key off the VERIFIED
+      // session, not a spoofable `x-user-id` header — otherwise anyone can pass
+      // the "must be signed in" check by inventing a user id.
+      const signedInUserId = (await ctxOf(req)).userId;
       const recipientEmail = typeof q.email === 'string' ? q.email : undefined;
       const providedPassword =
         typeof q.password === 'string'

packages/plugins/plugin-sharing/src/share-link-service.test.ts

@@ -163,4 +163,49 @@ describe('ShareLinkService', () => {
     ];
     expect(await service.resolveToken('expired-token-xyz-123')).toBeNull();
   });
+
+  // ── [Finding-2] verified-authz enforcement ────────────────────────────────
+  describe('authorization (Finding-2)', () => {
+    it('only the creator may revoke a link (a different user is denied)', async () => {
+      const link = await service.createLink(
+        { object: 'ai_conversations', recordId: 'c1', audience: 'link_only', permission: 'view' },
+        { userId: 'owner' },
+      );
+      // A different signed-in user cannot revoke someone else's link.
+      await expect(service.revokeLink(link.id, { userId: 'attacker' })).rejects.toMatchObject({ status: 403 });
+      // The row is untouched.
+      expect(engine._tables.sys_share_link[0].revoked_at).toBeNull();
+      // The creator can.
+      await expect(service.revokeLink(link.id, { userId: 'owner' })).resolves.toBeUndefined();
+      expect(engine._tables.sys_share_link[0].revoked_at).not.toBeNull();
+    });
+
+    it('a system/internal caller may revoke any link', async () => {
+      const link = await service.createLink(
+        { object: 'ai_conversations', recordId: 'c1', audience: 'link_only', permission: 'view' },
+        { userId: 'owner' },
+      );
+      await expect(service.revokeLink(link.id, { isSystem: true })).resolves.toBeUndefined();
+      expect(engine._tables.sys_share_link[0].revoked_at).not.toBeNull();
+    });
+
+    it('an HTTP caller cannot mint a link for a record it cannot see (403, not 404)', async () => {
+      // The record-access re-read runs under the caller context; a record the
+      // caller cannot see (here: does not exist) yields a fail-closed 403 for an
+      // untrusted caller — never a link, and without leaking existence.
+      await expect(
+        service.createLink(
+          { object: 'ai_conversations', recordId: 'ghost', audience: 'link_only', permission: 'view' },
+          { userId: 'u1' },
+        ),
+      ).rejects.toMatchObject({ status: 403 });
+      // A system caller still gets the plain 404.
+      await expect(
+        service.createLink(
+          { object: 'ai_conversations', recordId: 'ghost', audience: 'link_only', permission: 'view' },
+          { isSystem: true },
+        ),
+      ).rejects.toMatchObject({ status: 404 });
+    });
+  });
 });

packages/plugins/plugin-sharing/src/share-link-service.ts

@@ -234,16 +234,23 @@ export class ShareLinkService implements IShareLinkService {
       throw makeError(400, 'VALIDATION_FAILED', 'emailAllowlist is required when audience=email');
     }
 
-    // Confirm the target record actually exists — silently issuing
-    // links against ghost rows is a footgun.
+    // Confirm the target record exists AND — for an HTTP caller — that the
+    // caller may actually SEE it. [Finding-2] Reading under the caller's own
+    // context (positions/permissions/RLS) means you can only mint a link for a
+    // 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.
     const exists = await this.engine.find(input.object, {
       where: { id: input.recordId },
       fields: ['id'],
       limit: 1,
-      context: SYSTEM_CTX,
+      context: context.isSystem ? SYSTEM_CTX : context,
     } as any);
     if (!Array.isArray(exists) || exists.length === 0) {
-      throw makeError(404, 'RECORD_NOT_FOUND', `${input.object}/${input.recordId} does not exist`);
+      // Don't distinguish "missing" from "not visible" for an untrusted caller.
+      throw context.isSystem
+        ? makeError(404, 'RECORD_NOT_FOUND', `${input.object}/${input.recordId} does not exist`)
+        : makeError(403, 'FORBIDDEN', `Not permitted to share ${input.object}/${input.recordId}`);
     }
 
     const maxDays = policy.maxExpiryDays ?? DEFAULT_MAX_EXPIRY_DAYS;
@@ -277,17 +284,23 @@ export class ShareLinkService implements IShareLinkService {
     return row;
   }
 
-  async revokeLink(idOrToken: string, _context: ShareLinkExecutionContext): Promise<void> {
+  async revokeLink(idOrToken: string, context: ShareLinkExecutionContext): Promise<void> {
     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', {
       where: filter,
-      fields: ['id', 'revoked_at'],
+      fields: ['id', 'revoked_at', 'created_by'],
       limit: 1,
       context: SYSTEM_CTX,
     } as any);
     const row = Array.isArray(rows) ? (rows[0] as any) : undefined;
     if (!row) return; // No-op when missing
+    // [Finding-2] Only the link's creator may revoke it (internal/system callers
+    // bypass). Previously the caller context was ignored, so any client could
+    // revoke any user's link by id/token (sharing DoS).
+    if (!context.isSystem && row.created_by !== context.userId) {
+      throw makeError(403, 'FORBIDDEN', 'Not permitted to revoke this share link');
+    }
     if (row.revoked_at) return; // Already revoked
     await this.engine.update(
       'sys_share_link',

packages/plugins/plugin-sharing/src/sharing-plugin.ts

@@ -1,8 +1,9 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import type { Plugin, PluginContext } from '@objectstack/core';
+import { resolveAuthzContext } from '@objectstack/core';
 import type { EngineMiddleware, OperationContext } from '@objectstack/objectql';
-import type { IHttpServer } from '@objectstack/spec/contracts';
+import type { IHttpServer, IHttpRequest, ShareLinkExecutionContext } from '@objectstack/spec/contracts';
 import { SysRecordShare, SysSharingRule, SysShareLink } from './objects/index.js';
 import { SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity';
 import { SharingService, type SharingEngine } from './sharing-service.js';
@@ -288,8 +289,43 @@ export class SharingServicePlugin implements Plugin {
             // No HTTP server — service still reachable via getService.
           }
           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).
+            const ql: any = engine;
+            const verifiedContextFromRequest = async (req: IHttpRequest): Promise<ShareLinkExecutionContext> => {
+              try {
+                const headers = new Headers();
+                for (const [k, v] of Object.entries(req.headers ?? {})) {
+                  if (v == null) continue;
+                  headers.set(String(k), Array.isArray(v) ? v.join(',') : String(v));
+                }
+                const getSession = async (h: any) => {
+                  try {
+                    const authService: any = ctx.getService('auth');
+                    let api: any = authService?.api;
+                    if (!api && typeof authService?.getApi === 'function') api = await authService.getApi();
+                    return await api?.getSession?.({ headers: h });
+                  } catch {
+                    return undefined;
+                  }
+                };
+                const authz = await resolveAuthzContext({ ql, headers, getSession });
+                return {
+                  userId: authz.userId,
+                  tenantId: authz.tenantId,
+                  positions: authz.positions,
+                  permissions: authz.permissions,
+                };
+              } catch {
+                return {}; // anonymous → authed routes 401
+              }
+            };
             registerShareLinkRoutes(http, this.linkService, engine as SharingEngine, {
               basePath: this.options.shareLinkBasePath,
+              contextFromRequest: verifiedContextFromRequest,
             });
             ctx.logger.info(
               'SharingServicePlugin: share-link routes mounted at ' +

packages/spec/src/contracts/share-link-service.ts

@@ -101,6 +101,14 @@ export interface ShareLinkExecutionContext {
   userId?: string;
   tenantId?: string;
   isSystem?: boolean;
+  /**
+   * [Finding-2] The caller's resolved positions / permissions. Populated by the
+   * verified route wiring so RLS-aware checks (e.g. "can this caller actually
+   * read the record being shared?") evaluate against the real principal rather
+   * than a spoofable header identity.
+   */
+  positions?: string[];
+  permissions?: string[];
 }
 
 /**