feat(attachments): authenticated, parent-scoped downloads (#2970 item 2)

The storage download endpoints (GET /storage/files/:fileId and
/files/:fileId/url) were anonymous capability URLs — anyone holding a
fileId could mint a download with no session or access check.

For scope='attachments', non-public_read files, both endpoints now gate
on a new authorizeFileRead seam: 401 AUTH_REQUIRED without a session, 403
ATTACHMENT_DOWNLOAD_DENIED when the caller is neither the file owner nor
able to READ a record the file is attached to (parent-derived, resolved
through the full caller context via resolveAuthzContext), else a
short-lived signed URL (downloadTtl, default 300s). Non-attachments files
(field files, avatars, org logos — embedded in <img src> which can't
carry a bearer) keep the stable anonymous capability URL; bare
kernels/tests without the seam stay open (back-compat).

Tests: storage-routes gate unit tests (401/403/allow/public/non-attach/
open); the dogfood matrix (e-read) pin flips from a leak to a denial —
anon 401, cross-user 403, owner 200, parent-inherited 200, 302 anon 401.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187NT3Qer9oep5dCRb9b8Lt

Author: claude

.changeset/attachment-authenticated-downloads.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/service-storage': minor
+---
+
+feat(attachments): authenticated, parent-scoped downloads for attachments files (#2970)
+
+Closes item 2 of #2970. The storage download endpoints (`GET /storage/files/:fileId`
+and `/files/:fileId/url`) were anonymous capability URLs — anyone holding a
+`fileId` could mint a download without a session or any access check.
+
+For `scope === 'attachments'`, non-`public_read` files, both endpoints now gate
+on a new `authorizeFileRead` seam: `401 AUTH_REQUIRED` without a session, `403
+ATTACHMENT_DOWNLOAD_DENIED` when the caller is neither the file's owner nor able
+to READ a record the file is attached to (parent-derived, resolved through the
+full caller context via `resolveAuthzContext`), and otherwise a **short-lived**
+signed URL (`downloadTtl`, default 300s). Non-attachments files (field files,
+avatars, org logos — embedded in `<img src>` which cannot carry a bearer token)
+keep the stable anonymous capability URL, and bare kernels/tests without the
+seam wired stay open (back-compat).

packages/dogfood/test/attachments-permission-matrix.dogfood.test.ts

@@ -284,13 +284,41 @@ describe('attachments permission matrix (#2755)', () => {
     expect(okRows.some((r: any) => r.file_id === okFile)).toBe(true);
   });
 
-  // ── (e-read) known gap: anonymous downloads ──────────────────────────
-  it('(e-read, KNOWN GAP pin) anonymous download of a committed file id succeeds (capability URL)', async () => {
-    const fileId = await uploadFile(stack, memberATok);
-    const res = await stack.api(`/storage/files/${fileId}/url`);
-    // Anyone holding a fileId can mint a download URL without a session.
-    // Authenticated downloads / signed-link gating is a filed follow-up.
-    expect(res.status).toBe(200);
+  // ── (e-read) attachment downloads inherit parent visibility (#2970 item 2) ──
+  it('(e-read) attachments download requires auth AND read access to a parent record', async () => {
+    // A file attached to the admin-owned, private att_secret record.
+    const adminFile = await uploadFile(stack, adminTok);
+    const linked = await attach(adminTok, 'att_secret', secretId, adminFile);
+    expect(linked.status).toBeLessThan(300);
+
+    // Anonymous → 401 (was a 200 capability URL before #2970).
+    const anon = await stack.api(`/storage/files/${adminFile}/url`);
+    expect(anon.status).toBe(401);
+    expect(((await anon.json()) as any).code).toBe('AUTH_REQUIRED');
+
+    // memberB is authenticated but cannot read att_secret and is not the
+    // owner → 403.
+    const denied = await stack.apiAs(memberBTok, 'GET', `/storage/files/${adminFile}/url`);
+    expect(denied.status).toBe(403);
+    expect(((await denied.json()) as any).code).toBe('ATTACHMENT_DOWNLOAD_DENIED');
+
+    // The owner (admin) → 200 with a signed URL.
+    const owner = await stack.apiAs(adminTok, 'GET', `/storage/files/${adminFile}/url`);
+    expect(owner.status).toBe(200);
+    expect(((await owner.json()) as any).url).toBeTruthy();
+
+    // Parent-inherited read: a file on the PUBLIC att_case record is
+    // downloadable by any member who can read that record — even a
+    // non-uploader (memberB downloads memberA's attachment).
+    const caseFile = await uploadFile(stack, memberATok);
+    const caseLink = await attach(memberATok, 'att_case', caseAId, caseFile);
+    expect(caseLink.status).toBeLessThan(300);
+    const inherited = await stack.apiAs(memberBTok, 'GET', `/storage/files/${caseFile}/url`);
+    expect(inherited.status).toBe(200);
+
+    // The stable 302 endpoint is gated the same way (anon → 401).
+    const anon302 = await stack.api(`/storage/files/${adminFile}`);
+    expect(anon302.status).toBe(401);
   });
 
   // ── Part 1: sys_file orphan lifecycle, end to end ─────────────────────

packages/services/service-storage/src/index.ts

@@ -10,7 +10,7 @@ export type { S3StorageAdapterOptions } from './s3-storage-adapter.js';
 export { StorageMetadataStore } from './metadata-store.js';
 export type { FileRecord, UploadSessionRecord } from './metadata-store.js';
 export { registerStorageRoutes } from './storage-routes.js';
-export type { StorageRoutesOptions } from './storage-routes.js';
+export type { StorageRoutesOptions, FileReadVerdict } from './storage-routes.js';
 export { SystemFile, SystemUploadSession } from './objects/index.js';
 export { installAttachmentLifecycleHooks, createSysFileReapGuard } from './attachment-lifecycle.js';
 export type { AttachmentLifecycleEngine, AttachmentLifecycleLogger } from './attachment-lifecycle.js';

packages/services/service-storage/src/storage-routes.test.ts

@@ -250,6 +250,89 @@ describe('Storage REST Routes', () => {
     });
   });
 
+  describe('attachments download gate (#2970 item 2)', () => {
+    const commit = async (s: StorageMetadataStore, rec: Partial<import('./metadata-store').FileRecord>) =>
+      s.createFile({
+        id: rec.id ?? 'f-dl',
+        key: rec.key ?? `attachments/${rec.id ?? 'f-dl'}.bin`,
+        name: 'x.bin',
+        status: 'committed',
+        acl: rec.acl ?? 'private',
+        scope: rec.scope ?? 'attachments',
+        owner_id: rec.owner_id,
+        ...rec,
+      } as any);
+
+    function serverWith(verdict: import('./storage-routes').FileReadVerdict | 'skip', extra: any = {}) {
+      const server = createMockHttpServer();
+      const s = new StorageMetadataStore(null);
+      const authorizeFileRead =
+        verdict === 'skip' ? undefined : vi.fn(async () => verdict as any);
+      registerStorageRoutes(server as any, adapter, s, {
+        basePath: '/api/v1/storage',
+        authorizeFileRead,
+        ...extra,
+      });
+      return { server, store: s, authorizeFileRead };
+    }
+
+    const hit = async (server: any, path: string, fileId: string) => {
+      const res = createMockRes();
+      await server._getHandler('GET', path)!(createMockReq({ params: { fileId } }), res);
+      return res;
+    };
+
+    it('401s an unauthenticated download of an attachments file (both endpoints)', async () => {
+      const { server, store: s } = serverWith('unauthenticated');
+      await commit(s, { id: 'a1' });
+      for (const p of ['/api/v1/storage/files/:fileId/url', '/api/v1/storage/files/:fileId']) {
+        const res = await hit(server, p, 'a1');
+        expect(res._status, p).toBe(401);
+        expect(res._json?.code, p).toBe('AUTH_REQUIRED');
+      }
+    });
+
+    it('403s when the caller cannot read any parent record', async () => {
+      const { server, store: s } = serverWith('deny');
+      await commit(s, { id: 'a2' });
+      const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'a2');
+      expect(res._status).toBe(403);
+      expect(res._json?.code).toBe('ATTACHMENT_DOWNLOAD_DENIED');
+    });
+
+    it('allows the download when authorized, minting a short-lived signed URL', async () => {
+      const { server, store: s, authorizeFileRead } = serverWith('allow', { downloadTtl: 120 });
+      await commit(s, { id: 'a3' });
+      const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'a3');
+      expect(res._status).toBe(200);
+      expect(res._json.url).toContain('/_local/raw/');
+      expect(authorizeFileRead).toHaveBeenCalledOnce();
+    });
+
+    it('never gates a public_read attachments file (authorizer not consulted)', async () => {
+      const { server, store: s, authorizeFileRead } = serverWith('deny');
+      await commit(s, { id: 'a4', acl: 'public_read' });
+      const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'a4');
+      expect(res._status).toBe(200);
+      expect(authorizeFileRead).not.toHaveBeenCalled();
+    });
+
+    it('never gates a non-attachments file (avatars / field files stay open)', async () => {
+      const { server, store: s, authorizeFileRead } = serverWith('deny');
+      await commit(s, { id: 'a5', scope: 'user', key: 'user/a5.png' });
+      const res = await hit(server, '/api/v1/storage/files/:fileId', 'a5');
+      expect(res._status).toBe(302);
+      expect(authorizeFileRead).not.toHaveBeenCalled();
+    });
+
+    it('stays open when no authorizer is wired (back-compat)', async () => {
+      const { server, store: s } = serverWith('skip');
+      await commit(s, { id: 'a6' });
+      const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'a6');
+      expect(res._status).toBe(200);
+    });
+  });
+
   describe('PUT/GET /_local/raw/:token', () => {
     it('should accept raw upload with valid token and serve download', async () => {
       // Generate a presigned upload

packages/services/service-storage/src/storage-routes.ts

@@ -2,9 +2,12 @@
 
 import { randomUUID } from 'node:crypto';
 import type { IHttpServer, IHttpRequest, IHttpResponse, IStorageService } from '@objectstack/spec/contracts';
-import type { StorageMetadataStore } from './metadata-store.js';
+import type { StorageMetadataStore, FileRecord } from './metadata-store.js';
 import type { LocalStorageAdapter } from './local-storage-adapter.js';
 
+/** Authorization verdict for an attachments-scope download (#2970 item 2). */
+export type FileReadVerdict = 'allow' | 'deny' | 'unauthenticated';
+
 /**
  * Options for the storage route registration helper.
  */
@@ -24,6 +27,26 @@ export interface StorageRoutesOptions {
    * is a tracked follow-up needing cookie sessions or signed links).
    */
   resolveSession?: (req: IHttpRequest) => Promise<{ userId?: string } | null | undefined>;
+  /**
+   * Authorize a DOWNLOAD of an `attachments`-scope file (#2970 item 2). When
+   * wired, the download endpoints (`/files/:fileId` and `/files/:fileId/url`)
+   * consult this for `scope==='attachments'`, non-`public_read` files only:
+   *   - `unauthenticated` → 401 (no session)
+   *   - `deny` → 403 (session, but cannot read a parent record the file is
+   *     attached to and is not the owner)
+   *   - `allow` → a short-lived signed URL is issued
+   * Non-attachments files (field files, avatars, org logos) keep the stable
+   * anonymous capability URL — they are embedded in `<img src>` which cannot
+   * carry a bearer token, and are out of scope for the attachments leak.
+   * When absent (bare kernels, tests), all downloads stay open (back-compat).
+   */
+  authorizeFileRead?: (file: FileRecord, req: IHttpRequest) => Promise<FileReadVerdict>;
+  /**
+   * TTL (seconds) for the signed URL minted on a GATED attachments download.
+   * Short by design — the link is followed immediately after an explicit
+   * click. Default 300 (5 min). Non-gated downloads keep `presignedTtl`.
+   */
+  downloadTtl?: number;
   /** Optional logger for the one-time open-mode notice. */
   logger?: { info(msg: string): void; warn(msg: string): void };
 }
@@ -55,6 +78,40 @@ export function registerStorageRoutes(
   const basePath = opts.basePath ?? '/api/v1/storage';
   const presignedTtl = opts.presignedTtl ?? 3600;
   const sessionTtl = opts.sessionTtl ?? 86400;
+  const downloadTtl = opts.downloadTtl ?? 300;
+
+  // ── Download authorization gate (#2970 item 2) ───────────────────────
+  // Only `attachments`-scope, non-public files are gated; everything else
+  // keeps the stable anonymous capability URL (image/avatar embedding).
+  // Returns the signed-URL TTL to use, or `false` if a response was already
+  // sent (401/403) and the handler must stop.
+  const authorizeDownload = async (
+    file: FileRecord,
+    req: IHttpRequest,
+    res: IHttpResponse,
+  ): Promise<number | false> => {
+    if (file.scope !== 'attachments' || file.acl === 'public_read' || !opts.authorizeFileRead) {
+      return presignedTtl;
+    }
+    let verdict: FileReadVerdict;
+    try {
+      verdict = await opts.authorizeFileRead(file, req);
+    } catch {
+      verdict = 'deny'; // a failed authz check must never fall open
+    }
+    if (verdict === 'unauthenticated') {
+      res.status(401).json({ error: 'Authentication required to download this file', code: 'AUTH_REQUIRED' });
+      return false;
+    }
+    if (verdict === 'deny') {
+      res.status(403).json({
+        error: 'You do not have access to a record this file is attached to',
+        code: 'ATTACHMENT_DOWNLOAD_DENIED',
+      });
+      return false;
+    }
+    return downloadTtl;
+  };
 
   // ── Upload auth gate (#2755) ─────────────────────────────────────────
   // `false` ⇒ the 401 was already sent and the handler must stop.
@@ -431,12 +488,15 @@ export function registerStorageRoutes(
         return;
       }
 
+      const ttl = await authorizeDownload(file, req, res);
+      if (ttl === false) return;
+
       let url: string;
       if (storage.getPresignedDownload) {
-        const desc = await storage.getPresignedDownload(file.key, presignedTtl);
+        const desc = await storage.getPresignedDownload(file.key, ttl);
         url = desc.downloadUrl;
       } else if (storage.getSignedUrl) {
-        url = await storage.getSignedUrl(file.key, presignedTtl);
+        url = await storage.getSignedUrl(file.key, ttl);
       } else {
         url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`;
       }
@@ -467,12 +527,15 @@ export function registerStorageRoutes(
         return;
       }
 
+      const ttl = await authorizeDownload(file, req, res);
+      if (ttl === false) return;
+
       let url: string;
       if (storage.getPresignedDownload) {
-        const desc = await storage.getPresignedDownload(file.key, presignedTtl);
+        const desc = await storage.getPresignedDownload(file.key, ttl);
         url = desc.downloadUrl;
       } else if (storage.getSignedUrl) {
-        url = await storage.getSignedUrl(file.key, presignedTtl);
+        url = await storage.getSignedUrl(file.key, ttl);
       } else {
         url = `${basePath}/_local/file/${encodeURIComponent(file.key)}`;
       }