Repository navigation
fix(objectql): a file field reads the same whether or not the projection lists id - #26938
Conversation
…er the projection lists id A reader refused sys_file read is served a file field by the parent-derived verdict, matched on the owning record's id. find/findOne now read id for any projection that names a file field and cut it back off the answer with the formula widening, so select=<file field> answers what select=id,<file field> does, and a refused reader stays refused. Claude-Session: https://claude.ai/code/session_01FEkQ5nso8iiU7cfQSEAkX5 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5nso8iiU7cfQSEAkX5 Co-authored-by: Claude <noreply@anthropic.com>
…d door test Claude-Session: https://claude.ai/code/session_01FEkQ5nso8iiU7cfQSEAkX5 Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 19 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b97f94e7810af641f38d8e61639253a5cd8b6411 && git checkout b97f94e7810af641f38d8e61639253a5cd8b6411
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a2818c57a21c96a1634e1598cb82066bc320a65 6ea02bc0b048573946633ae4d7aa63803ce782d3 && git checkout -B drift-repro 7a2818c57a21c96a1634e1598cb82066bc320a65 && git merge --no-ff 6ea02bc0b048573946633ae4d7aa63803ce782d3
node scripts/docs-audit/affected-docs.mjs --json 7a2818c57a21c96a1634e1598cb82066bc320a65
|
席位核对 · ACCEPT ·
|
| item | reading |
|---|---|
| Shape | Draft, base main, Fixes #26672 under triage 6442544054 (p3, under Leehom's 「p1 p2 处理完了,就可以处理 p3」 2026-10-26T10:29Z). 5 files: engine.ts (withFileOwnerKey, called in find and findOne), an extended objectql hydration test, a new REST door test, one door-agreement pin, and a patch changeset on @objectstack/objectql. Clause-②: no and Metadata-protocol: none appear in both the body and the changeset. |
| Fix | A reader without sys_file read is served a file field by the owning record's verdict, matched on that record's id. A projection that left id out gave the verdict no key, so a permitted file came back { id, metadataRefused: true }. Now an explicit projection that names a file-reference field and not id also reads id, and the existing formula-widening cut (withoutFormulaWidening, at the return, after the middleware chain) trims it, so the answer has exactly the named columns. The verdict, the refused marker and a sys_file reader's own read are unchanged. Every door shares it through engine.find/findOne: REST list, query, by-id, and the dispatcher. |
| Seat reading | The added id is a read the caller's row already allowed; nothing new is served beyond what select=id,FILE_FIELD already served. A refused reader stays refused with or without id (pinned). |
| Evidence | CI green on head 6ea02bc0b0 (34 success, 11 skipped, 0 failed). Governed check: NOT governed, 331 changed lines. Red first on main: 3 of 6 REST door tests red (the three id-less doors), 6 of 6 green after; the engine.ts ablation turns the objectql id-less case red while the refused control stays green, restored to the HEAD blob. objectql 8,850 and rest 6,519 tests pass; 67 of 67 derived gates exit 0, --ran 0 NOT-MEASURED. |
| Overlap | The head merge-tree is clean against main. Open PRs that also edit engine.ts, #26932 (the #25522 split's count cut) and #26930, each merge-tree clean against this head. |
| Landing | 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay, once every check on this head is green or an expected skip. |
Generated by Claude Code
Requested by Leehom · project thread
Fixes #26672
Clause-②: no
Metadata-protocol: none
维护者速读
find/findOne在调用方投影里点名了文件字段却没点名id时,内部多读一列id,读完后再把它从答案里裁掉(与公式字段扩宽列的裁剪同一处)。sys_file的读者,文件元数据按"拥有该文件的记录"的判决来给,而这个判决按记录id匹配。投影漏了id,判决就找不到记录,于是有权读的应用管理员在select=FILE_FIELD时拿到metadataRefused: true,加上id却拿到完整值(hotclm 报告)。id的投影,多读一列、答案列不变;无权读者照样被拒(两种投影都钉了)。回滚即还原packages/objectql/src/engine.ts的一处函数和两处调用。packages/objectql/src/engine.ts(withFileOwnerKey及find/findOne两处调用)、packages/rest/src/data-file-field-projection-without-id.test.ts、packages/objectql/src/engine-file-hydrate-parent-derived.test.ts(改写了原先钉住旧行为的用例)。pnpm --filter @objectstack/rest exec vitest run src/data-file-field-projection-without-id.test.ts,预期 6 个用例全绿;在 main 上同一命令 3 红 3 绿(红的是三个"不带 id"的门)。What changes
packages/objectql/src/engine.ts: a new module helperwithFileOwnerKey(schema, requestedFields, planned)runs right afterplanFormulaProjectionin bothfindandfindOne. When the caller's explicit projection names a file-reference field (FILE_REFERENCE_TYPES) and the planned projection does not carryid, it appendsidto what the driver is asked for and addsidto the widened set. The existingwithoutFormulaWidening/rowsWithoutFormulaWideningcut at thereturn(after the middleware chain) then trims it, so the answer carries exactly the columns the caller named. Every consumer in between (formulas,expand, file-reference hydration,afterFindhooks, the secret mask, the middlewares' post-phase) reads the row with itsid, which is whatresolveFileReferences→serveRefusedFileHydration→readParentDerivedFilesneed to match a file to the record that owns it.sys_fileread are untouched: a reader the field-owned verdict refuses still gets{ id, metadataRefused: true }, a copied-in id is still refused on the record it was copied into.readParentDerivedFilesno longer cites "a projection that left outid" as a reason for an empty map, andwithoutFormulaWideningnames the second thing it cuts.@objectstack/objectqlpatch.Which doors share the fix
The fix is in the engine's two read verbs, so every door that reads through them shares it; no door holds its own copy of the hydration.
GET /api/v1/data/:object?select=crud-routes.ts→protocol.findData→engine.findPOST /api/v1/data/:object/queryfieldscrud-routes.ts→protocol.findData→engine.findGET /api/v1/data/:object/:id?select=crud-routes.ts→protocol.getData→engine.findOneruntime/src/domains/data.ts→action-execution.tscallData→protocol.findData/protocol.getData→ the same engine verbsexpandsub-readsexpandRelatedRecords→this.findwith the join keyidalready added to a nested projectionid; now also covered by the engine ruleengine.find/engine.findOnewithfieldsTests
packages/rest/src/data-file-field-projection-without-id.test.tson the composed REST harness (RestServer→ObjectStackProtocolImplementation→ObjectQL→SqlDriveron better-sqlite3), a reader refusedsys_fileread whose field-owned verdict allows it:GET /data/:object?select=attachment,POST /data/:object/query { fields: ['attachment'] },GET /data/:object/:id?select=attachment: the full file value, and noidkey on the row.select=id,attachment(all three doors): unchanged.{ id, metadataRefused: true }with and withoutid, all three doors.packages/objectql/src/engine-file-hydrate-parent-derived.test.ts: the case "a read whose projection leaves out id cannot name its owner record, so every id keeps the marker" pinned exactly the defect this card fixes; it now pins that such a read gets the same verdict asfields: ['id', 'attachment'](owner hydrates, a copied-in id stays refused, rows carry onlyattachment), onfindandfindOne, plus a new refused-reader control.packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts, against the REAL field-owned verdictStorageServicePluginhands the engine: an id-less projection hydrates the owned file and the delegate-allowed file, and keeps the marker for the delegate's deny and for a copied-in id the reader cannot read.Acceptance notes
idis read for any explicit projection naming a file field, whichever arm the hydration takes (the caller's ownsys_fileread does not need it). Narrowing it to the refused arm would need the admission answer before the driver read; one extra column on such reads was preferred over a second planning path.afterFindhook on such a read now seesidon the row, and has it cut afterwards, exactly as for a projection naming a formula field (objectql: a formula field in afieldsprojection widens it to every column and the rows are never trimmed back — get_record sends owner, org and audit columns to an external endpoint #22300).Verification
All readings below are on head
6ea02bc0unless a line says otherwise.Red first on main, green after (REST door test)
main(1708115e, objectqldist/built from main):pnpm --filter @objectstack/rest exec vitest run src/data-file-field-projection-without-id.test.tsanswered 3 failed, 3 passed. The three reds are the three id-less doors:expected [ { attachment: { …(2) } } ] to deeply equal [ { attachment: { …(5) } } ](list and query) andexpected { id: 'f_26672_owned', …(1) } to deeply equal { id: 'f_26672_owned', …(4) }(by-id). The three greens are the refusal it starts from, theselect=id,…control and the refused-reader control.Ablation (objectql suite, resolves src)
engine.tsreplaced on disk with itsmainblob under a restoringtrap(mutantwithFileOwnerKeycount 0):engine-file-hydrate-parent-derived.test.tsanswered 1 failed, 16 passed, the red being the id-less case; the refused-reader control stayed green, as it should. Restore proven by the HEAD blob hash and an emptygit diff HEAD(RESTORED-OK).Package suites and typecheck
pnpm --filter @objectstack/objectql test: Test Files 475 passed (475), Tests 8850 passed (8850).pnpm --filter @objectstack/rest test: Test Files 329 passed (329), Tests 6519 passed, 326 skipped (6845).pnpm --filter @objectstack/service-storage test: Test Files 70 passed (70), Tests 1258 passed (1258).pnpm --filter @objectstack/objectql --filter @objectstack/rest --filter @objectstack/service-storage run typecheck: exit 0 (each package'scheck:test-typecheckOK).845325d3; the only later change is oneas anyremoved from the new REST test, re-run on6ea02bc0withresttypecheck and the file (6 passed).eslint --no-inline-configoverpackages/objectql/src, the new REST test andpackages/services/service-storage/src: exit 0.Derived gates (dispatch-gates --commands, 67 families)
Run on
6ea02bc0, every exit code captured before any pipe: 67 of 67 exit 0.dispatch-gates --ranreconciles: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN. The first run on845325d3was red oncheck:query-options-erasure(the new test's directsys_fileread carried anas any, test surface 236 → 237); the cast was removed, so the test surface is back at 236.test:repo(the vitestrepoprojects) was not run locally: the diff touches no file in anyvitest.repo-tests.jsonand the derived list names none; CI's Test Core runs it.Dogfood battery: not run. The title carries no
!andClause-②has no(narrowing)arm.Generated by Claude Code