Skip to content

Commit 134df4f

Browse files
os-zhuangclaude
andauthored
feat(storage): governed download for field-owned files + R4 reconciliation gate — ADR-0104 D3 wave 2 (PR-4) (#3534)
* 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 * feat(storage): verifyFileReferences — the executable R4 acceptance gate ADR-0104 D3 wave 2 planned to gate enabling collection on "reference counting is verified", which is aspirational rather than checkable. This makes it runnable. verifyFileReferences() compares GROUND TRUTH (what records' file-class fields actually hold) against RECORDED OWNERSHIP (sys_file.ref_object/ref_id/ref_field) and classifies every disagreement by whether it can cause DATA LOSS once collection is on, because only that class should block a release: blocking unowned_reference a record holds a file nothing owns — with collection on it looks free and its bytes go foreign_owner a record holds a file owned by a different slot; that reference is invisible to the lifecycle, so the bytes go when the recorded owner releases shared_reference one file held by two slots — exclusivity was violated, so copy-on-claim did not run, and one file's ACL is serving two parents advisory stale_owner owned but no longer held; fails toward retention unreferenced_file committed and unpointed-at — storage cost, not risk The scan is read-only: it never writes, tombstones or deletes. The point is that a ledger may not be handed authority over an irreversible delete until it has been shown to agree with reality, so this is meant to be run repeatedly, on real tenant data, and to report zero blocking discrepancies on consecutive runs before the gated collection change may merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2fa4ca1 commit 134df4f

7 files changed

Lines changed: 808 additions & 24 deletions

File tree

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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.
34+
35+
---
36+
37+
Also adds `verifyFileReferences()` — the executable form of ADR-0104's R4
38+
acceptance gate. It compares ground truth (what records' file fields actually
39+
hold) against recorded ownership, and classifies disagreements by whether they
40+
could cause data loss once collection is enabled:
41+
42+
- **blocking**`unowned_reference` (a held file nothing owns), `foreign_owner`
43+
(a record holds a file owned by another slot), `shared_reference` (one file
44+
held by two slots, i.e. exclusivity was violated). Each would let a later reap
45+
delete bytes a record still points at.
46+
- **advisory**`stale_owner` (owned but no longer held; fails toward
47+
retention) and `unreferenced_file` (storage cost, not a correctness problem).
48+
49+
The scan is read-only — it never writes, tombstones, or deletes. A ledger may
50+
not be given authority over irreversible deletes until it has been shown to
51+
agree with reality, so this must report zero blocking discrepancies on real
52+
tenant data, on consecutive runs, before the gated collection change may merge.

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,17 @@ export { installAttachmentLifecycleHooks, createSysFileReapGuard, createUploadSe
1616
export type { AttachmentLifecycleEngine, AttachmentLifecycleLogger } from './attachment-lifecycle.js';
1717
export { installFileReferenceHooks, FileReferenceCopyError } from './file-reference-lifecycle.js';
1818
export type { FileReferenceEngine, FileReferenceLogger } from './file-reference-lifecycle.js';
19+
export {
20+
verifyFileReferences,
21+
formatFileReferenceReport,
22+
BLOCKING_ISSUE_KINDS,
23+
} from './verify-file-references.js';
24+
export type {
25+
FileReferenceIssue,
26+
FileReferenceIssueKind,
27+
FileReferenceReport,
28+
VerifyReferencesEngine,
29+
VerifyReferencesOptions,
30+
} from './verify-file-references.js';
1931
export { installAttachmentAccessHooks, installAttachmentReadVisibility } from './attachment-access-hooks.js';
2032
export type { AttachmentSharingLike } from './attachment-access-hooks.js';

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)