Skip to content

Commit f70b28b

Browse files
committed
feat(storage): governed download for field-owned files — ADR-0104 D3 wave 2 (PR-4)
A file owned by a record's field (sys_file.ref_object / ref_id, set by PR-3) is now authorized on download the same way an attachment is: the caller must be able to READ the file's parent record, or be its uploader. Previously only attachments-scope files were gated and every field file kept an anonymous capability URL. Parent resolution differs by surface, and that asymmetry is the point of the ownership model. An attachment may hang off many records, so its readable-by set is the union over its sys_attachment join rows. A field-owned file belongs to exactly one record, so its readable-by set is that one record's and nothing more. Under a shared reference model the field case would have had to union too — which is exactly how copying a file id into a more public record would silently widen access. Denials report FILE_DOWNLOAD_DENIED, distinct from the attachments path's ATTACHMENT_DOWNLOAD_DENIED: the file BELONGS TO one record rather than being ATTACHED TO several, and the message should not claim otherwise. acl: 'public_read' is the opt-out, and now an explicit declaration rather than the silent default every field file used to get. Genuinely public images — anything embedded in an <img src>, which cannot carry a bearer token — must declare it. Gates nothing that is open today. A pre-cutover field holds an inline blob or an external URL, never a sys_file id, so no existing file has an owner recorded and none of them start being gated; the gate engages only for files a field has actually claimed, and disengages when ownership is released. Both are covered by regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
1 parent 99736a0 commit f70b28b

4 files changed

Lines changed: 173 additions & 24 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/service-storage": minor
3+
---
4+
5+
feat(storage): governed download for field-owned files — ADR-0104 D3 wave 2 (PR-4)
6+
7+
A file owned by a record's field (`sys_file.ref_object` / `ref_id`, set by
8+
PR-3) is now authorized on download the same way an attachment is: the caller
9+
must be able to READ the file's parent record, or be its uploader. Previously
10+
only `attachments`-scope files were gated and every field file kept an
11+
anonymous capability URL.
12+
13+
**Parent resolution differs by surface, and that asymmetry is the point.** An
14+
attachment may hang off many records, so its readable-by set is the union over
15+
its `sys_attachment` join rows. A field-owned file belongs to exactly one
16+
record, so its readable-by set is that one record's — nothing more. Under a
17+
shared reference model the field case would have had to union too, which is
18+
what makes copying a file id into a more public record silently widen access.
19+
20+
Denials are reported as `FILE_DOWNLOAD_DENIED` (403), distinct from the
21+
attachments path's `ATTACHMENT_DOWNLOAD_DENIED`, since the file *belongs to* one
22+
record rather than being *attached to* several.
23+
24+
**`acl: 'public_read'` is the opt-out**, and now an explicit declaration rather
25+
than the silent default every field file used to get. Genuinely public images —
26+
anything embedded in an `<img src>`, which cannot carry a bearer token — must
27+
declare it.
28+
29+
**Dual-mode safe, gates nothing that is open today.** A pre-cutover field holds
30+
an inline blob or an external URL, never a `sys_file` id, so no existing file
31+
has an owner recorded and none of them start being gated. The gate engages only
32+
for files a record's field has actually claimed, and disengages again when
33+
ownership is released.

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

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ describe('Storage REST Routes', () => {
320320
expect(authorizeFileRead).not.toHaveBeenCalled();
321321
});
322322

323-
it('never gates a non-attachments file (avatars / field files stay open)', async () => {
323+
it('never gates a file with neither an attachments scope nor a field owner', async () => {
324324
const { server, store: s, authorizeFileRead } = serverWith('deny');
325325
await commit(s, { id: 'a5', scope: 'user', key: 'user/a5.png' });
326326
const res = await hit(server, '/api/v1/storage/files/:fileId', 'a5');
@@ -334,6 +334,75 @@ describe('Storage REST Routes', () => {
334334
const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'a6');
335335
expect(res._status).toBe(200);
336336
});
337+
338+
// ── Field-owned files (ADR-0104 D3 wave 2) ─────────────────────
339+
describe('field-owned files', () => {
340+
const owned = (id: string, over: any = {}) => ({
341+
id,
342+
scope: 'user',
343+
key: `user/${id}.png`,
344+
ref_object: 'product',
345+
ref_id: 'p1',
346+
ref_field: 'image',
347+
...over,
348+
});
349+
350+
it('gates a field-owned file even though its scope is not attachments', async () => {
351+
const { server, store: s, authorizeFileRead } = serverWith('deny');
352+
await commit(s, owned('fo1'));
353+
const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'fo1');
354+
expect(res._status).toBe(403);
355+
// Distinct from the attachments code — this file BELONGS to one
356+
// record, it is not attached to several.
357+
expect(res._json?.code).toBe('FILE_DOWNLOAD_DENIED');
358+
expect(authorizeFileRead).toHaveBeenCalledOnce();
359+
});
360+
361+
it('401s an unauthenticated field-owned download', async () => {
362+
const { server, store: s } = serverWith('unauthenticated');
363+
await commit(s, owned('fo2'));
364+
const res = await hit(server, '/api/v1/storage/files/:fileId', 'fo2');
365+
expect(res._status).toBe(401);
366+
expect(res._json?.code).toBe('AUTH_REQUIRED');
367+
});
368+
369+
it('allows an authorized field-owned download', async () => {
370+
const { server, store: s } = serverWith('allow', { downloadTtl: 120 });
371+
await commit(s, owned('fo3'));
372+
const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'fo3');
373+
expect(res._status).toBe(200);
374+
});
375+
376+
it('opts out via acl: public_read (the embedding escape hatch)', async () => {
377+
const { server, store: s, authorizeFileRead } = serverWith('deny');
378+
await commit(s, owned('fo4', { acl: 'public_read' }));
379+
const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'fo4');
380+
expect(res._status).toBe(200);
381+
expect(authorizeFileRead).not.toHaveBeenCalled();
382+
});
383+
384+
/**
385+
* DUAL-MODE REGRESSION. A pre-cutover field holds an inline blob or an
386+
* external URL, never a sys_file id — so no legacy file is ever claimed,
387+
* `ref_object` stays unset, and this change gates nothing that used to
388+
* be open. An unclaimed upload behaves the same way.
389+
*/
390+
it('does not gate an unclaimed file (no ref_object) — legacy fields keep working', async () => {
391+
const { server, store: s, authorizeFileRead } = serverWith('deny');
392+
await commit(s, { id: 'fo5', scope: 'user', key: 'user/fo5.png' });
393+
const res = await hit(server, '/api/v1/storage/files/:fileId', 'fo5');
394+
expect(res._status).toBe(302);
395+
expect(authorizeFileRead).not.toHaveBeenCalled();
396+
});
397+
398+
it('does not gate a file whose ownership was released', async () => {
399+
const { server, store: s, authorizeFileRead } = serverWith('deny');
400+
await commit(s, owned('fo6', { ref_object: null, ref_id: null, ref_field: null }));
401+
const res = await hit(server, '/api/v1/storage/files/:fileId', 'fo6');
402+
expect(res._status).toBe(302);
403+
expect(authorizeFileRead).not.toHaveBeenCalled();
404+
});
405+
});
337406
});
338407

339408
describe('PUT/GET /_local/raw/:token', () => {

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

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,20 @@ export interface StorageRoutesOptions {
2727
* is a tracked follow-up needing cookie sessions or signed links).
2828
*/
2929
resolveSession?: (req: IHttpRequest) => Promise<{ userId?: string } | null | undefined>;
30-
/**
31-
* Authorize a DOWNLOAD of an `attachments`-scope file (#2970 item 2). When
32-
* wired, the download endpoints (`/files/:fileId` and `/files/:fileId/url`)
33-
* consult this for `scope==='attachments'`, non-`public_read` files only:
30+
/**
31+
* Authorize a DOWNLOAD of a parent-governed file (#2970 item 2, extended by
32+
* ADR-0104 D3 wave 2). When wired, the download endpoints
33+
* (`/files/:fileId` and `/files/:fileId/url`) consult this for
34+
* `scope==='attachments'` files AND for field-owned files (`ref_object` set),
35+
* excluding `public_read` ones:
3436
* - `unauthenticated` → 401 (no session)
35-
* - `deny` → 403 (session, but cannot read a parent record the file is
36-
* attached to and is not the owner)
37+
* - `deny` → 403 (session, but cannot read the parent record the file
38+
* belongs to / is attached to, and is not the owner)
3739
* - `allow` → a short-lived signed URL is issued
38-
* Non-attachments files (field files, avatars, org logos) keep the stable
39-
* anonymous capability URL — they are embedded in `<img src>` which cannot
40-
* carry a bearer token, and are out of scope for the attachments leak.
40+
* A file with neither an attachments scope nor a field owner — an unclaimed
41+
* upload, an org logo — keeps the stable anonymous capability URL, as does
42+
* any file explicitly marked `acl: 'public_read'` (the opt-in for genuinely
43+
* public embedding, since `<img src>` cannot carry a bearer token).
4144
* When absent (bare kernels, tests), all downloads stay open (back-compat).
4245
*/
4346
authorizeFileRead?: (file: FileRecord, req: IHttpRequest) => Promise<FileReadVerdict>;
@@ -80,17 +83,30 @@ export function registerStorageRoutes(
8083
const sessionTtl = opts.sessionTtl ?? 86400;
8184
const downloadTtl = opts.downloadTtl ?? 300;
8285

83-
// ── Download authorization gate (#2970 item 2) ───────────────────────
84-
// Only `attachments`-scope, non-public files are gated; everything else
85-
// keeps the stable anonymous capability URL (image/avatar embedding).
86+
// ── Download authorization gate (#2970 item 2, ADR-0104 D3 wave 2) ───
87+
// Two kinds of file are gated, both deriving access from a PARENT record:
88+
// - `attachments`-scope files, via their sys_attachment join rows;
89+
// - field-owned files, via the single record whose field owns them
90+
// (`ref_object`/`ref_id`, ADR-0104 D3 wave 2).
91+
// `acl: 'public_read'` opts a file back out to the stable anonymous
92+
// capability URL — needed for genuinely public embedding (`<img src>`
93+
// cannot carry a bearer token), and now an explicit declaration rather
94+
// than the silent default it used to be for every field file.
95+
//
96+
// Dual-mode safe: a legacy field holds an inline blob or an external URL,
97+
// never a `sys_file` id, so no legacy file has `ref_object` set and none of
98+
// them start being gated by this change.
99+
//
86100
// Returns the signed-URL TTL to use, or `false` if a response was already
87101
// sent (401/403) and the handler must stop.
88102
const authorizeDownload = async (
89103
file: FileRecord,
90104
req: IHttpRequest,
91105
res: IHttpResponse,
92106
): Promise<number | false> => {
93-
if (file.scope !== 'attachments' || file.acl === 'public_read' || !opts.authorizeFileRead) {
107+
const fieldOwned = !!file.ref_object && file.ref_id != null && file.ref_id !== '';
108+
const gated = file.scope === 'attachments' || fieldOwned;
109+
if (!gated || file.acl === 'public_read' || !opts.authorizeFileRead) {
94110
return presignedTtl;
95111
}
96112
let verdict: FileReadVerdict;
@@ -104,10 +120,17 @@ export function registerStorageRoutes(
104120
return false;
105121
}
106122
if (verdict === 'deny') {
107-
res.status(403).json({
108-
error: 'You do not have access to a record this file is attached to',
109-
code: 'ATTACHMENT_DOWNLOAD_DENIED',
110-
});
123+
res.status(403).json(
124+
fieldOwned
125+
? {
126+
error: 'You do not have access to the record this file belongs to',
127+
code: 'FILE_DOWNLOAD_DENIED',
128+
}
129+
: {
130+
error: 'You do not have access to a record this file is attached to',
131+
code: 'ATTACHMENT_DOWNLOAD_DENIED',
132+
},
133+
);
111134
return false;
112135
}
113136
return downloadTtl;

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

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -463,12 +463,20 @@ function buildAuthSessionResolver(
463463
}
464464

465465
/**
466-
* Authorize an `attachments`-scope download (#2970 item 2). Builds the FULL
467-
* caller ExecutionContext via `resolveAuthzContext` (the same shared resolver
468-
* rest-server uses — a bare `{ userId }` would lack the resolved permissions
469-
* the parent RLS needs), then allows when the caller is the file's owner or
470-
* can READ a record the file is attached to. Returns `undefined` (routes stay
471-
* open) when the auth service or engine is absent.
466+
* Authorize a parent-governed download (#2970 item 2; extended for field-owned
467+
* files by ADR-0104 D3 wave 2). Builds the FULL caller ExecutionContext via
468+
* `resolveAuthzContext` (the same shared resolver rest-server uses — a bare
469+
* `{ userId }` would lack the resolved permissions the parent RLS needs), then
470+
* allows when the caller is the file's owner or can READ the file's parent
471+
* record. Returns `undefined` (routes stay open) when the auth service or
472+
* engine is absent.
473+
*
474+
* "Parent" resolves differently for the two surfaces, and that asymmetry is the
475+
* point of the ownership model: an attachment may hang off MANY records, so its
476+
* readable-by set is the union over its join rows; a field-owned file belongs
477+
* to exactly ONE record, so its readable-by set is that record's and nothing
478+
* more. A shared model would have had to union field references too, silently
479+
* widening access whenever one file id was copied into a more public record.
472480
*/
473481
function buildFileReadAuthorizer(
474482
ctx: PluginContext,
@@ -487,6 +495,22 @@ function buildFileReadAuthorizer(
487495
// Uploader / owner may always download.
488496
if (file.owner_id && String(file.owner_id) === String(authz.userId)) return 'allow';
489497

498+
// Field-owned (ADR-0104 D3 wave 2): exactly ONE record's field holds
499+
// this file, so access is that record's READ access — never a union.
500+
if (file.ref_object && file.ref_id != null && file.ref_id !== '') {
501+
try {
502+
const visible = (await (engine as any).find(String(file.ref_object), {
503+
where: { id: file.ref_id },
504+
fields: ['id'],
505+
limit: 1,
506+
context: authz,
507+
})) as Array<Record<string, unknown>>;
508+
return visible?.length ? 'allow' : 'deny';
509+
} catch {
510+
return 'deny'; // unknown/failing owner object — fail closed
511+
}
512+
}
513+
490514
// Otherwise: readable via any parent record this file is attached to.
491515
const links = (await (engine as any).find('sys_attachment', {
492516
where: { file_id: file.id },

0 commit comments

Comments
 (0)