Skip to content

fix(objectql): a file field reads the same whether or not the projection lists id - #26938

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-dywp1n
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-dywp1n

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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 helper withFileOwnerKey(schema, requestedFields, planned) runs right after planFormulaProjection in both find and findOne. When the caller's explicit projection names a file-reference field (FILE_REFERENCE_TYPES) and the planned projection does not carry id, it appends id to what the driver is asked for and adds id to the widened set. The existing withoutFormulaWidening / rowsWithoutFormulaWidening cut at the return (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, afterFind hooks, the secret mask, the middlewares' post-phase) reads the row with its id, which is what resolveFileReferences → serveRefusedFileHydration → readParentDerivedFiles need to match a file to the record that owns it.
  • The parent-derived verdict, the refused marker and the caller's own sys_file read 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.
  • Docblocks: readParentDerivedFiles no longer cites "a projection that left out id" as a reason for an empty map, and withoutFormulaWidening names the second thing it cuts.
  • Changeset @objectstack/objectql patch.

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.

Door Path to the hydration Shares the fix
REST GET /api/v1/data/:object?select= crud-routes.ts → protocol.findData → engine.find yes (pinned)
REST POST /api/v1/data/:object/query fields crud-routes.ts → protocol.findData → engine.find yes (pinned)
REST GET /api/v1/data/:object/:id?select= crud-routes.ts → protocol.getData → engine.findOne yes (pinned)
Dispatcher data domain (list, query, by-id) runtime/src/domains/data.ts → action-execution.ts callData → protocol.findData / protocol.getData → the same engine verbs yes (same call, read not re-pinned)
expand sub-reads expandRelatedRecords → this.find with the join key id already added to a nested projection already carried id; now also covered by the engine rule
In-process engine.find / engine.findOne with fields direct yes (pinned in objectql and service-storage)

Tests

  • New packages/rest/src/data-file-field-projection-without-id.test.ts on the composed REST harness (RestServer → ObjectStackProtocolImplementation → ObjectQL → SqlDriver on better-sqlite3), a reader refused sys_file read 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 no id key on the row.
    • CONTROL select=id,attachment (all three doors): unchanged.
    • CONTROL a caller the verdict refuses: { id, metadataRefused: true } with and without id, all three doors.
  • Changed 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 as fields: ['id', 'attachment'] (owner hydrates, a copied-in id stays refused, rows carry only attachment), on find and findOne, plus a new refused-reader control.
  • Added case in packages/services/service-storage/src/field-owned-file-hydration-door-agreement.test.ts, against the REAL field-owned verdict StorageServicePlugin hands 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

Verification

All readings below are on head 6ea02bc0 unless a line says otherwise.

Red first on main, green after (REST door test)
  • On main (1708115e, objectql dist/ built from main): pnpm --filter @objectstack/rest exec vitest run src/data-file-field-projection-without-id.test.ts answered 3 failed, 3 passed. The three reds are the three id-less doors: expected [ { attachment: { …(2) } } ] to deeply equal [ { attachment: { …(5) } } ] (list and query) and expected { id: 'f_26672_owned', …(1) } to deeply equal { id: 'f_26672_owned', …(4) } (by-id). The three greens are the refusal it starts from, the select=id,… control and the refused-reader control.
  • After the fix (objectql rebuilt): 6 passed (6).
Ablation (objectql suite, resolves src)

engine.ts replaced on disk with its main blob under a restoring trap (mutant withFileOwnerKey count 0): engine-file-hydrate-parent-derived.test.ts answered 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 empty git 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's check:test-typecheck OK).
  • Those suites ran on 845325d3; the only later change is one as any removed from the new REST test, re-run on 6ea02bc0 with rest typecheck and the file (6 passed).
  • eslint --no-inline-config over packages/objectql/src, the new REST test and packages/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 --ran reconciles: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN. The first run on 845325d3 was red on check:query-options-erasure (the new test's direct sys_file read carried an as any, test surface 236 → 237); the cast was removed, so the test surface is back at 236.

test:repo (the vitest repo projects) was not run locally: the diff touches no file in any vitest.repo-tests.json and the derived list names none; CI's Test Core runs it.

Dogfood battery: not run. The title carries no ! and Clause-② has no (narrowing) arm.


Generated by Claude Code

…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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 2 documentable anchor(s).

14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-flow.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/automation/hook-bodies.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/automation/webhooks.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/kernel/contracts/data-engine.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/kernel/contracts/index.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/kernel/events.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/permissions/attachments-access.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/permissions/field-level-security.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/permissions/record-view-auditing.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/permissions/rls.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/permissions/system-context.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/protocol/objectql/query-syntax.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/protocol/objectql/schema.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/ui/react-pages.mdx (via findOne (symbol, a method of class ObjectQL))

⛔ 7 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v15.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/releases/v16.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/releases/v17/17-0.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/releases/v17/17-5.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/releases/v17/17-6.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/releases/v17/17-7.mdx (via findOne (symbol, a method of class ObjectQL))
  • content/docs/releases/v17/index.mdx (via findOne (symbol, a method of class ObjectQL))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 72 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 55 of 207 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 19 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 7a2818c57a21c96a1634e1598cb82066bc320a65 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b97f94e7810af641f38d8e61639253a5cd8b6411 — the merge of head 6ea02bc0b048573946633ae4d7aa63803ce782d3 into base 7a2818c57a21c96a1634e1598cb82066bc320a65, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 7a2818c57a21c96a1634e1598cb82066bc320a65 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:server seat 1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC · 2026-10-27T22:43Z

Head 6ea02bc0b0, report 6454991916 (on #26672), claim 6453303667. No contract review is owed: there is no packages/spec/** diff, no governed path, no narrowing and no Clause-②: yes.

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

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 27, 2026 22:44
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 27, 2026 22:44
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 1f1eda4 Oct 27, 2026
46 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-dywp1n branch October 27, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A REST read that projects a file field without id answers {id, metadataRefused: true} for a caller that may read the file

2 participants