Skip to content

Commit d9e6ac2

Browse files
fix(storage)!: a file its record lets go keeps that record's read rule (#26144)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XGuJWm35AkoM8dcbRg59uXx)_ Fixes #26046 Clause-②: no (narrowing) Metadata-protocol: none ## What this changes ADR-0104 D3 promises one file, one owning record and one access rule. This makes the rule hold after the owning record lets the file go. Until a writer who may read that record claims the file again, or the platform collects it, the file is read by that record's rule. - **Release keeps the record** (`file-reference-lifecycle.ts`). `sys_file` gains two nullable bookkeeping fields, `released_ref_object` and `released_ref_id`, naming the record that last owned the file. Every release writes them, whether or not it tombstones. A claim clears them, because the claiming record's rule replaces the old one. - **One verdict reads them** (`judgeFileRead` in `storage-service-plugin.ts`). Such a file is judged by that record's read access, through `readableFieldOwners`, the function the field-owned arm already asks (so a `fileAccessDelegate` decides here too). The download door and the claim path (#25692) share this verdict, so both follow it. A record that no longer exists is read by nobody, so only the uploader reads its released files. - **Unchanged.** A file no record has owned is read within its organization (#25693). The uploader always reads their own file. `public_read`, attachments-scope files, #25692's write rule and #26036's anonymous-write rule are untouched. The reap guard still reads only the ownership columns, so the new fields never hold a file back from collection. #26043's path is not touched. - **ADR-0104**: one line in the 2026-10-26 addendum's list of who the verdict admits. It changes D3's text, so this PR is Tier H and stays draft for the maintainer. The measurements and the exact path are with the server seat, not here. ## Declarations - `Clause-②: no (narrowing)`, measured on this diff: a file read that the owning record's rule refuses is now refused after the record lets the file go. Nothing refused becomes accepted. The claim wrote `Clause-②: no` and named this arm as expected. - `Metadata-protocol: none`: nothing a metadata author writes changes. The two fields are platform-managed bookkeeping on a platform object. - One `patch` changeset on `@objectstack/service-storage`, `fix(storage)!:`, with the BREAKING section and the ADR-0087 disposition `not-required (no-migration-prescription)`. - Existing rows: schema sync adds the two columns, existing rows read NULL, and a NULL row is judged exactly as before. No migration is needed for the shape. ## Tests HEAD `7cc88889e`. - New `file-reference-release-read-rule.integration.test.ts`: one real `ObjectQL` + sqlite engine with `StorageServicePlugin` booted as shipped, asked through the download door and the claim path, for every way a file leaves its slot, under both collection settings. On base `0a6948210` it is red (13 failed, 8 passed); on this branch 21/21 pass. - Ablation: the new verdict arm disabled through `scripts/ablation-replace.mjs` (anchor 1 to 0, blob `24540c7fe6b3` to `12afcb53c051`) turns the same 13 red; restored, blob equals HEAD and `git diff HEAD` is empty. - `pnpm --filter @objectstack/service-storage test`: 66 files, 1218 tests passed. `typecheck` (including `check:test-typecheck`) passed. eslint on `packages/services/service-storage` with `--no-inline-config`: clean. - Full dogfood battery at HEAD: `pnpm --filter @objectstack/dogfood test` under the verify lock, VERDICT command-exit 0, Test Files 296 passed | 3 skipped (299), Tests 2523 passed | 18 skipped (2541). ## Gates Local subset at HEAD `7cc88889e`: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 80 families from this diff; all 80 ran with exit 0 (each code captured before any pipe), and `--ran` reconciles 80 run, 0 NOT MEASURED. Before the full build, `check:i18n` and `check:dual-build-cjs-loads` answered exit 3 (prerequisite not met) and `check-plugin-teardown-shape --self-test` exit 3 (shallow clone); after `pnpm build` and fetching the self-test's control commit all three are 0, and `check:i18n` drove the translation-bundle commit. `node scripts/check-platform-checklist.mjs --changed-from origin/main`: OK, 4 anchors judged, 0 lost. Every other family is CI's; this PR's check-runs are the record. ## Acceptance notes - **Rows that predate this change.** One residual about them is reported to the server seat with the measurements, for the maintainer to weigh. No follow-up card is filed by this PR. - **Signed-URL lifetime.** The download door still mints a released file's URL with the presigned TTL of an unowned file, not the shorter TTL of a record-governed one (`authorizeDownload`'s `gated` test reads the ownership columns only). That decides how long an already-issued URL lives, not who gets one. - `#26043 remains open`; its path is not touched. ## 维护者速读(草稿) **改了什么**:记录放手一个文件后,这个文件继续按原记录的读权限判定,直到有能读原记录的人重新认领它,或它被平台回收。`sys_file` 加了两个平台自管的空值字段记住"原记录";ADR-0104 的 2026-10-26 补记里改了一行。 **为什么改**:ADR-0104 承诺"一个文件、一个所属记录、一条访问规则"。实测发现这条规则在记录放手文件之后不成立(细节只在席位线程)。合同、证件、HR 附件这类私密记录里的文件,替换或删除后也要保持原来的访问范围。 **风险与代价(含回滚)**:这是收窄:以前能读到的这类文件,现在不能读原记录的成员读不到了(上传者本人始终能读)。修复前已有的数据有一处遗留,详情在席位线程。回滚即 revert 本 PR;两个新列留在库里无害。 **席位意见**: **你要做的**:审阅 ADR 那一行与修复,批准后合并(Tier H,保持 draft 等你)。 --- _Generated by [Claude Code](https://claude.ai/code/session_018e5yJf1ptbMURVgWb3LrF6)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018e5yJf1ptbMURVgWb3LrF6 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 601c773 commit d9e6ac2

11 files changed

Lines changed: 495 additions & 4 deletions
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/service-storage': patch
3+
---
4+
5+
fix(storage)!: a file its record lets go keeps that record's read rule
6+
7+
ADR-0104 D3 promises one file, one owning record and one access rule. That rule now holds after the owning record lets the file go: until a writer who may read that record claims the file again, or the platform collects it, the file is read by that record's rule. The download door and the record-write claim path ask the same verdict, so both follow it. A file no record has owned is read as before, within its organization. The uploader still reads their own file.
8+
9+
`sys_file` gains two nullable fields, `released_ref_object` and `released_ref_id`, naming the record that last owned the file. The platform writes them when a record lets a file go and clears them when a record claims it. Existing rows hold NULL and are read as before.
10+
11+
Clause-②: no (narrowing) — a file read that a record's rule refuses is now refused after the record lets the file go. Nothing that was refused becomes accepted.
12+
13+
Metadata-protocol: none (a file read rule and two platform-managed bookkeeping fields; nothing a metadata author writes changes)
14+
15+
<!-- adr-0087: not-required (no-migration-prescription) No metadata author's surface moves: no spec key or authorable spelling is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. The platform object `sys_file` gains two nullable bookkeeping fields that only the platform writes; schema sync adds the columns and existing rows read NULL, which is the shape every row had before. What changes is which callers a file read admits after the file's owning record lets it go. The remedy is reading the file as a reader of that record, never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
16+
17+
**BREAKING** for a reader the owning record's rule refuses, shipped as `patch` because the metadata protocol does not move.
18+
19+
- **Before.** The owning record's rule did not hold for the file once the record let it go.
20+
- **Now.** It is read by the rule of the record that let it go, until it is claimed again or collected.
21+
- **Fix.** Read or claim such a file as a reader of that record.

‎docs/adr/0104-field-runtime-value-shape-contract.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,9 +1459,11 @@ exists in the acting organization. One file had two access rules.
14591459
- **One judgement, two callers.** The door's verdict is one function, and the
14601460
claim path calls it under the write's caller context; neither restates it.
14611461
It admits the uploader; a reader of the record that owns the file, through
1462-
its object's `fileAccessDelegate` where one is declared; a member of the
1463-
organization for a file no record owns; a reader of any parent for an
1464-
attachments-scope file; anyone for a `public_read` file.
1462+
its object's `fileAccessDelegate` where one is declared; for a file a record
1463+
let go, a reader of that record, until the file is claimed again or
1464+
collected (#26046); a member of the organization for a file no record has
1465+
owned; a reader of any parent for an attachments-scope file; anyone for a
1466+
`public_read` file.
14651467
- **The quiet answer.** At the write door, a file the writer may not read is
14661468
answered as one the organization-scoped lookup cannot see: the id is kept
14671469
as written, nothing is claimed or copied, and constraint verdicts do not run

‎packages/services/service-storage/src/file-reference-lifecycle.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,17 @@ import { callerContext } from './attachment-access-hooks.js';
6565
* and keep any other id as written; neither is a file the anonymous caller
6666
* may provide. An empty or absent file field is written as before.
6767
*
68+
* ### A released file keeps its record's rule
69+
*
70+
* [#26046] Releasing a file clears its ownership columns, and the download
71+
* door reads a file no record owns by the organization's rule. That rule is
72+
* for a file no record ever held; a file a record let go is still the one that
73+
* record held. So release keeps the record ({@link releaseOwnership}), the
74+
* door's verdict judges the file by that record's read access until it is
75+
* claimed again or collected, and a claim, which asks the same verdict, is
76+
* open only to a writer who may read that record. A deleted record is read by
77+
* nobody, so its released files are left to their uploader.
78+
*
6879
* ### The one write that cannot honour it, and is refused
6980
*
7081
* [#7102] A predicate (`multi: true`) update has ONE payload for N matched
@@ -803,6 +814,12 @@ async function claimFile(
803814
patch.status = 'committed';
804815
patch.deleted_at = null;
805816
}
817+
// [#26046] The claiming record's rule replaces the one the file kept from
818+
// the record that let it go.
819+
if (row.released_ref_object != null || row.released_ref_id != null) {
820+
patch.released_ref_object = null;
821+
patch.released_ref_id = null;
822+
}
806823
await engine.update('sys_file', patch, { context: { ...SYSTEM_CTX } });
807824
}
808825

@@ -848,6 +865,13 @@ async function collectionOpen(engine: FileReferenceEngine): Promise<boolean> {
848865
* Only a `committed` file is tombstoned: `pending` rows already carry their
849866
* own never-completed reap policy, and re-tombstoning a `deleted` row would
850867
* reset its grace clock.
868+
*
869+
* [#26046] Every release keeps the record that let the file go
870+
* (`released_ref_object` / `released_ref_id`), whether or not it tombstones:
871+
* the file is read by that record's rule until a writer who may read it
872+
* claims it again ({@link claimFile} clears them), or it is collected. Without
873+
* them a released file would read as one no record ever held, whose rule is
874+
* the organization's, which is weaker than the record's was.
851875
*/
852876
async function releaseOwnership(
853877
engine: FileReferenceEngine,
@@ -861,7 +885,14 @@ async function releaseOwnership(
861885
if (id == null) continue;
862886
const tombstone = collect && row.status === 'committed';
863887
try {
864-
const patch: Record<string, unknown> = { id, ref_object: null, ref_id: null, ref_field: null };
888+
const patch: Record<string, unknown> = {
889+
id,
890+
ref_object: null,
891+
ref_id: null,
892+
ref_field: null,
893+
released_ref_object: row.ref_object ?? null,
894+
released_ref_id: row.ref_id ?? null,
895+
};
865896
if (tombstone) {
866897
patch.status = 'deleted';
867898
patch.deleted_at = new Date().toISOString();

0 commit comments

Comments
 (0)