Skip to content

Commit 274b894

Browse files
authored
Merge pull request #2969 from objectstack-ai/claude/attachments-v1-followups-8jhvin
feat(attachments): sys_file orphan lifecycle + non-admin permission matrix (#2755)
2 parents c64ee8c + 86c0aea commit 274b894

28 files changed

Lines changed: 1910 additions & 133 deletions
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
'@objectstack/objectql': minor
3+
'@objectstack/service-storage': minor
4+
'@objectstack/platform-objects': minor
5+
'@objectstack/verify': minor
6+
'@objectstack/rest': patch
7+
---
8+
9+
feat(attachments): sys_file orphan lifecycle + parent-derived attachment access (#2755)
10+
11+
**Orphan lifecycle (ADR-0057).** Deleting a `sys_attachment` join row used to
12+
orphan the backing `sys_file` row and its storage bytes forever. `sys_file`
13+
now declares a lifecycle (`ttl 30d` on a new `deleted_at` tombstone for
14+
orphans; `retention 7d onlyWhen status=pending` for abandoned uploads), the
15+
storage plugin's new hooks tombstone a file when its LAST join row is deleted
16+
(attachments scope only — `Field.file`/`Field.image`/avatar scopes are never
17+
touched) and un-tombstone on re-attach, and a new LifecycleService **reap
18+
guard** seam (`registerReapGuard`) re-verifies zero references at sweep time
19+
and deletes the storage bytes before confirming each row reap. A guarded
20+
object is never blind-deleted; an erroring guard fails safe (rows retained).
21+
22+
**Attachment access (ADR-0049, Salesforce parent-derived semantics).**
23+
`sys_attachment` create now requires caller READ visibility of the parent
24+
record (403 `ATTACHMENT_PARENT_ACCESS`) and server-stamps `uploaded_by` from
25+
the session (client value ignored); delete requires uploader-or-parent-editor
26+
(403 `ATTACHMENT_DELETE_DENIED`). The storage upload routes require an
27+
authenticated session when an auth service is wired (401 `AUTH_REQUIRED`;
28+
bare kernels stay open) and stamp `owner_id` on new files.
29+
30+
**REMOVED — `sys_attachment.share_type` / `sys_attachment.visibility`.**
31+
Both fields were modeled in v1 with zero runtime consumers (ADR-0049
32+
parsed-but-unenforced). There is no replacement key: attachment access is
33+
derived from the parent record by the hooks above. Writers of these fields
34+
should simply stop sending them (unknown-field validation will reject them);
35+
existing DB columns are left as unmanaged leftovers, no migration needed.
36+
37+
`@objectstack/verify` gains `BootOptions.extraPlugins` for booting optional
38+
service pairs (e.g. storage + audit) in dogfood fixtures.

docs/adr/0057-system-data-lifecycle-and-retention.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,28 @@ registered ⇒ rows are retained (today's behavior), reported as
170170
`archive-pending` — a compliance ledger cannot be destroyed by declaring a
171171
lifecycle.
172172

173+
#### Amendment (#2755): reap guards — domain re-verification + external-resource reclaim
174+
175+
Some reapable rows are pointers to state the Reaper cannot see: `sys_file`
176+
rows reference storage bytes (disk/S3), and a tombstoned orphan may have
177+
regained `sys_attachment` references since it was marked. For these, a plugin
178+
may register a **reap guard** at runtime
179+
(`lifecycle.registerReapGuard(object, guard)`): the Reaper fetches candidate
180+
rows in batches, the guard confirms each id — performing external cleanup
181+
(byte deletion) and sweep-time re-verification first — or vetoes it (kept,
182+
retried next sweep). Rules:
183+
184+
- A guarded object is **never blind-deleted**: an erroring guard fails safe
185+
(rows retained), and an engine without row reads skips the object
186+
(`reap-guard-unsupported`) rather than degrading to a blind delete.
187+
- Guards are runtime wiring, not spec surface — detection and scheduling stay
188+
inside the single platform sweep (no bespoke sweepers, per the alternatives
189+
below); the guard is a domain callback, not a scheduler.
190+
- First consumer: `sys_file` (service-storage) — attachment orphans are
191+
tombstoned by hooks when their last `sys_attachment` reference is deleted,
192+
reaped `30d` later by TTL with byte reclaim, with zero-reference
193+
re-verification at sweep time; abandoned `pending` uploads reap after `7d`.
194+
173195
### 3.4 Reclaim — driver space hygiene
174196

175197
SQLite driver defaults to `auto_vacuum=INCREMENTAL` (shipped P0); the Reaper

packages/dogfood/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
"@objectstack/example-crm": "workspace:*",
1313
"@objectstack/example-showcase": "workspace:*",
1414
"@objectstack/objectql": "workspace:*",
15+
"@objectstack/plugin-audit": "workspace:*",
1516
"@objectstack/plugin-auth": "workspace:*",
1617
"@objectstack/plugin-security": "workspace:*",
18+
"@objectstack/service-storage": "workspace:*",
1719
"@objectstack/spec": "workspace:*",
1820
"@objectstack/verify": "workspace:*"
1921
},

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

Lines changed: 468 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Attachments permission-matrix fixture (#2755).
4+
//
5+
// Three tiny objects spanning the enforcement axes of the generic
6+
// Attachments surface (#2727):
7+
//
8+
// att_case — enable.files + public sharing model: any member can read
9+
// (and per Salesforce "can edit parent ⇒ can manage its
10+
// attachments", any member can detach from it).
11+
// att_secret — enable.files + DEFAULT sharing model (omitted ⇒ private,
12+
// ADR-0090) + an `owner_id` field: the sharing service
13+
// read-filters rows to the owner, so a fresh member cannot
14+
// read (→ cannot attach to) another user's record, and
15+
// `canEdit` denies non-owners (→ cannot delete another
16+
// user's attachments on it).
17+
// att_nofiles — NO enable.files: the #2727 opt-in gate (FILES_DISABLED).
18+
//
19+
// No custom SecurityPlugin: a fresh signUp member falls back to the real
20+
// `member_default` wildcard-CRUD set — exactly the posture the issue's
21+
// permission matrix questions (the gates under test are the ones layered on
22+
// TOP of that wildcard).
23+
24+
import { defineStack } from '@objectstack/spec';
25+
import { ObjectSchema, Field } from '@objectstack/spec/data';
26+
import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security';
27+
import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security';
28+
29+
export const AttCase = ObjectSchema.create({
30+
name: 'att_case',
31+
label: 'Attachment Case',
32+
pluralLabel: 'Attachment Cases',
33+
sharingModel: 'public_read_write',
34+
enable: { files: true },
35+
fields: {
36+
name: Field.text({ label: 'Name', required: true }),
37+
},
38+
});
39+
40+
export const AttSecret = ObjectSchema.create({
41+
name: 'att_secret',
42+
label: 'Attachment Secret',
43+
pluralLabel: 'Attachment Secrets',
44+
// sharingModel omitted — custom object defaults to PRIVATE (ADR-0090);
45+
// owner_id is the sharing service's owner anchor.
46+
enable: { files: true },
47+
fields: {
48+
name: Field.text({ label: 'Name', required: true }),
49+
owner_id: Field.text({ label: 'Owner' }),
50+
},
51+
});
52+
53+
export const AttNoFiles = ObjectSchema.create({
54+
name: 'att_nofiles',
55+
label: 'No Files Here',
56+
pluralLabel: 'No Files Here',
57+
sharingModel: 'public_read_write',
58+
fields: {
59+
name: Field.text({ label: 'Name', required: true }),
60+
},
61+
});
62+
63+
/**
64+
* The domain grant a real app ships when it turns the attachments panel on
65+
* for members: `member_default` (the `everyone` anchor baseline) carries NO
66+
* `allowDelete` (ADR-0090 D5 — delete is not a baseline right), so managing
67+
* attachments requires an ordinary position-distributed set with the delete
68+
* bit on `sys_attachment`. The matrix grants this to some members and pins
69+
* the no-grant baseline with another.
70+
*/
71+
export const attachmentManagerSet: PermissionSet = PermissionSetSchema.parse({
72+
name: 'att_attachment_manager',
73+
label: 'Attachments Fixture — attachment manager',
74+
objects: {
75+
sys_attachment: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
76+
},
77+
});
78+
79+
/** SecurityPlugin carrying the platform defaults + the fixture's domain set. */
80+
export function attachmentsFixtureSecurity(): SecurityPlugin {
81+
return new SecurityPlugin({
82+
defaultPermissionSets: [...securityDefaultPermissionSets, attachmentManagerSet],
83+
});
84+
}
85+
86+
export const attachmentsFixtureStack = defineStack({
87+
manifest: {
88+
id: 'com.dogfood.attachments_fixture',
89+
namespace: 'att',
90+
version: '0.0.0',
91+
type: 'app',
92+
name: 'Attachments Permission Matrix Fixture',
93+
description:
94+
'Three-object app exercising the #2755 attachment permission matrix: parent visibility, uploader/editor delete, enable.files gate.',
95+
},
96+
objects: [AttCase, AttSecret, AttNoFiles],
97+
});

packages/objectql/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export type {
6363
LifecycleObjectLike,
6464
LifecycleSettingsLike,
6565
LifecycleGovernanceAlert,
66+
LifecycleReapGuard,
6667
} from './lifecycle/lifecycle-service.js';
6768
export { parseLifecycleDuration } from './lifecycle/duration.js';
6869
export { lifecycleSettingsManifest } from './lifecycle/lifecycle-settings.js';

packages/objectql/src/lifecycle/lifecycle-service.test.ts

Lines changed: 110 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@ function captureEngine(
1717
deleteImpl?: (object: string, options: any) => any;
1818
driver?: Record<string, unknown>;
1919
datasources?: Record<string, unknown>;
20+
/** When set, the engine exposes `find` (guarded-reap candidate reads). */
21+
findImpl?: (object: string, options: any) => any;
2022
} = {},
2123
) {
2224
const deletes: Array<{ object: string; where: any; multi: any; context: any }> = [];
23-
const engine = {
25+
const finds: Array<{ object: string; where: any; limit: any; context: any }> = [];
26+
const engine: any = {
2427
registry: { getAllObjects: () => objects },
2528
async delete(object: string, options: any) {
2629
deletes.push({ object, where: options?.where, multi: options?.multi, context: options?.context });
@@ -33,7 +36,13 @@ function captureEngine(
3336
return ds;
3437
},
3538
};
36-
return { engine, deletes };
39+
if (opts.findImpl) {
40+
engine.find = async (object: string, options: any) => {
41+
finds.push({ object, where: options?.where, limit: options?.limit, context: options?.context });
42+
return opts.findImpl!(object, options);
43+
};
44+
}
45+
return { engine, deletes, finds };
3746
}
3847

3948
function service(engine: any, extra: Partial<ConstructorParameters<typeof LifecycleService>[0]> = {}) {
@@ -307,6 +316,105 @@ describe('LifecycleService.sweep — Reaper', () => {
307316
});
308317
});
309318

319+
describe('LifecycleService.sweep — reap guard', () => {
320+
const guarded: LifecycleObjectLike[] = [
321+
{ name: 'sys_file', lifecycle: { class: 'transient', ttl: { field: 'deleted_at', expireAfter: '30d' } } },
322+
];
323+
324+
it('deletes only guard-confirmed ids, per id, after fetching candidates with the cutoff filter', async () => {
325+
const rows = [
326+
{ id: 'f1', deleted_at: '2020-01-01T00:00:00Z' },
327+
{ id: 'f2', deleted_at: '2020-01-02T00:00:00Z' },
328+
{ id: 'f3', deleted_at: '2020-01-03T00:00:00Z' },
329+
];
330+
const { engine, deletes, finds } = captureEngine(guarded, { findImpl: () => rows });
331+
const svc = service(engine);
332+
const guard = vi.fn(async () => ['f1', 'f3']); // vetoes f2
333+
svc.registerReapGuard('sys_file', guard);
334+
335+
const report = await svc.sweep();
336+
337+
expect(finds).toHaveLength(1);
338+
expect(finds[0].where).toEqual({ deleted_at: { $lt: isoCutoff('30d') } });
339+
expect(finds[0].context).toEqual({ isSystem: true, positions: [], permissions: [] });
340+
expect(guard).toHaveBeenCalledWith('sys_file', rows);
341+
// Per-id deletes — the engine's delete path reads `where.id` as a scalar
342+
// target; a `{$in}` there would be bound as an object by the driver.
343+
expect(deletes.map((d) => d.where)).toEqual([{ id: 'f1' }, { id: 'f3' }]);
344+
expect(report.swept).toEqual([
345+
{ object: 'sys_file', class: 'transient', policy: 'ttl', cutoff: isoCutoff('30d'), deleted: 2 },
346+
]);
347+
expect(report.errors).toEqual([]);
348+
});
349+
350+
it('an erroring guard fails safe: no rows deleted, error reported', async () => {
351+
const { engine, deletes } = captureEngine(guarded, { findImpl: () => [{ id: 'f1' }] });
352+
const svc = service(engine);
353+
svc.registerReapGuard('sys_file', async () => {
354+
throw new Error('storage unreachable');
355+
});
356+
357+
const report = await svc.sweep();
358+
359+
expect(deletes).toHaveLength(0);
360+
expect(report.swept).toEqual([]);
361+
expect(report.errors).toEqual([{ object: 'sys_file', error: 'storage unreachable' }]);
362+
});
363+
364+
it('a guard on one object never changes the blind reap of others (regression pin)', async () => {
365+
const { engine, deletes } = captureEngine(
366+
[
367+
...guarded,
368+
{ name: 'sys_job_run', lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } },
369+
],
370+
{ findImpl: () => [] },
371+
);
372+
const svc = service(engine);
373+
svc.registerReapGuard('sys_file', async () => []);
374+
375+
const report = await svc.sweep();
376+
377+
// sys_file: no candidates → no delete. sys_job_run: classic blind reap.
378+
expect(deletes).toHaveLength(1);
379+
expect(deletes[0].object).toBe('sys_job_run');
380+
expect(deletes[0].where).toEqual({ created_at: { $lt: isoCutoff('30d') } });
381+
expect(report.errors).toEqual([]);
382+
});
383+
384+
it('a guarded object on an engine without find is skipped, never blind-deleted', async () => {
385+
const { engine, deletes } = captureEngine(guarded); // no findImpl → no engine.find
386+
const svc = service(engine);
387+
svc.registerReapGuard('sys_file', async () => ['f1']);
388+
389+
const report = await svc.sweep();
390+
391+
expect(deletes).toHaveLength(0);
392+
expect(report.swept).toEqual([]);
393+
expect(report.skipped).toEqual([{ object: 'sys_file', reason: 'reap-guard-unsupported' }]);
394+
});
395+
396+
it('drains full batches but stops the pass when a batch is not fully confirmed', async () => {
397+
// Two "pages" of 500, then a short page. All of page 1 confirmed → loop
398+
// continues; page 2 only partially confirmed → pass ends (vetoed rows
399+
// would be re-fetched forever within one sweep).
400+
const page = (n: number, prefix: string) =>
401+
Array.from({ length: n }, (_, i) => ({ id: `${prefix}${i}`, deleted_at: '2020-01-01T00:00:00Z' }));
402+
const pages = [page(500, 'a'), page(500, 'b')];
403+
let call = 0;
404+
const { engine, deletes, finds } = captureEngine(guarded, { findImpl: () => pages[call++] ?? [] });
405+
const svc = service(engine);
406+
svc.registerReapGuard('sys_file', async (_object, rows) =>
407+
rows[0].id === 'a0' ? rows.map((r: any) => r.id) : rows.slice(0, 10).map((r: any) => r.id),
408+
);
409+
410+
const report = await svc.sweep();
411+
412+
expect(finds).toHaveLength(2);
413+
expect(deletes).toHaveLength(510); // 500 confirmed from page 1 + 10 from page 2, per id
414+
expect(report.swept[0].deleted).toBe(510);
415+
});
416+
});
417+
310418
describe('LifecycleService.sweep — Archiver (P3)', () => {
311419
const AUDIT_OBJ: LifecycleObjectLike = {
312420
name: 'sys_audit_log',

0 commit comments

Comments
 (0)