Repository navigation
feat(lint): security-external-owner-unmapped warns when a writable external object refuses every member write - #27422
Conversation
… write An external object on an owner-keyed sharing model whose owner column no remote row serves refuses every member edit and delete (403): record sharing reads owner_id, the ownership floor reads created_by == caller, and neither has storage. security-external-owner-unmapped is an advisory on os validate / build / lint naming the two ways out (public_read_write plus a beforeUpdate hook, or owner_id mapped through external.columnMap). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
The new advisory rule raises the count authoringRulesFor('validate') and
('build') derive; check:docs-transcript-drift holds the transcripts to it.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
…umnMap external.columnMap is applied by the built-in SQL drivers, not by the engine, so on a plugin driver a columnMap entry for owner_id serves no check. The warning is now silenced only by a declared owner_id (or created_by on an object without owner_id), words a columnMap-only entry on its own, and names the way out driver-neutrally: declare the field and have the driver return the remote owner under that name. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
📓 Docs Drift CheckThis PR changes 1 package(s): 51 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 13 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 4 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 dee780778641558ffea39b5306ed2faffab9357b && git checkout dee780778641558ffea39b5306ed2faffab9357b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c5d46341747746b180626ad2fcb918324341cd22 120864f47554b20338ec27a4c5e4a6249ebe8ca4 && git checkout -B drift-repro c5d46341747746b180626ad2fcb918324341cd22 && git merge --no-ff 120864f47554b20338ec27a4c5e4a6249ebe8ca4
node scripts/docs-audit/affected-docs.mjs --json c5d46341747746b180626ad2fcb918324341cd22
|
Review nits: the verdict now says a member write is refused unless an app-authored row-level policy admits the row (sharing's admitsByIdWrite retracts its refusal for such a row); the created_by hint drops the audit-columns clause (a declared created_by suffices with audit off); a datasource the stack declares with external.allowWrites: false is skipped. The fix line is pinned whole in the test. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
|
epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV 维护者速读
席位意见:ACCEPT。独立 Fable 完整复核 ACCEPT-with-nits,三处措辞/范围已在 席位核对
|
Requested by 田騏 · project thread
Fixes #27332
Clause-②: no
Metadata-protocol: none
维护者速读
os validate/os build/os lint新增一条警告security-external-owner-unmapped:可写的外部对象若用按所有者的共享模型(private、public_read,或不写),又没在fields声明所有者检查读的那一列,就提示;警告点名两条出路(改public_read_write由beforeUpdatehook 判谁能改;或在fields声明owner_id,并让驱动以这个名字返回远端所有者——内置 SQL 驱动靠external.columnMap做到,插件驱动要自己返回,因为columnMap不由引擎执行)。allowEdit的普通成员改任何一行、删任何一行都被 403(共享检查读owner_id,所有权兜底读created_by == 当前用户,远端行都没有这两列),以前os validate不提示。实测删除同样被拒;ownership: 'none'不是出路(只是把拒绝从共享检查挪到兜底),所以它照样报警,改为点名created_by。defineStack和所有发布入口接受的集合不变;运行时零改动。回滚 = revert 本 PR。packages/lint/src/validate-external-owner-sharing.ts(判定的两条分支undeclaredOwnershipColumn,以及只有columnMap没声明字段时的措辞)、packages/lint/src/rule-explanations.ts(os explain的长文)、packages/lint/src/authoring-rules.ts(登记为 advisory、仅 CLI)。pnpm --filter @objectstack/lint exec vitest run src/validate-external-owner-sharing.test.ts,预期 14 个用例全过:spike 的spike_owned报一条警告,spike_node(public_read_write)不报,且在os validate的结果里是 advisory 不是 error。What changes
A new advisory rule,
security-external-owner-unmapped, on all three authoring commands (validate,build,lint; CLI only, held off the runtime publish door for the reason every advisory object rule carries,RUNTIME_OBJECT_ADVISORY_VOLUME).It warns on an object that has an
externalbinding withwritable: true, an effective sharing model that keys writes to an owner (private,public_read, or none on a custom object, which the runtime treats asprivate), and no remote column serving the check that refuses the write:owner_id(ownership: 'user', the default) as the platform's injected, unstored anchor, not declared infields. Record sharing readsowner_id, gets null, and refuses.owner_id(ownership: 'none' | 'org' | 'business_unit'). Record sharing then abstains, and the platform ownership floor (created_by == current_user.id) decides. It warns whencreated_byis the unstored anchor, not declared, or when the audit columns are off (the floor then compiles to a deny).What lets the owner write is the driver returning the remote owner under the field name the check reads.
external.columnMapis applied by the built-in SQL drivers, not by the engine, so on a plugin driver acolumnMapentry serves nothing. The author's vouch the rule reads is therefore a declaredowner_id/created_byfield (the provenance the runtime guards read:resolveInjectedColumnProvenance,@objectstack/spec/data); acolumnMapentry alone still warns, worded for that case (works on a built-in SQL driver only). The message names the column the refusing check reads; the hint names both ways out, driver-neutrally:public_read_writeplus abeforeUpdatehook, or declare the field and have the driver return the remote owner under that name (a built-in SQL driver throughexternal.columnMap, a plugin driver by itself). This matches the guide wording on PR #27403 ("have your driver return the owner under the field name the checks read"). The long text (both checks, whyownership: 'none'is no way out, deletes) isos explain security-external-owner-unmapped, and agrees with the guide's "Choose the sharing model on purpose" callout (content/docs/data-modeling/virtual-objects.mdx, not edited here).Supporting edits: the effective-sharing-model mirror in
validate-sharing-rule-enforceability.tsis exported and reused instead of copied a second time; the rule id is exported from the root barrel; one changeset (@objectstack/lintpatch).Outside
packages/lint: the rule count in four CLI docs transcripts (55 to 56, see notes). Not touched: the runtime,defineStack, the spike test (the pin lives in the lint test, with the spike's two object shapes copied verbatim), the virtual-objects guide.Delete (the card's third task)
Yes, the same two reads refuse
delete. Measured over the spike's plugin-driver composition (a one-off harness, not committed):DELETE /data/OBJECT/IDby a member holdingallowDeleteanswers 403PERMISSION_DENIEDon the default,privateandpublic_readmodels, and the remote is asked exactlyfind { where: { id }, fields: ['id', 'owner_id'], limit: 1 }thenfindOne { where: { $and: [{ id }, { created_by: CALLER }] }, limit: 1 };driver.deleteis never called.One more fact the PR names because the warning's first way out depends on it: on
public_read_writethe edit passes (200) but the delete is still refused (403). The floor keeps deletes tocreated_bythere by design (owdOpenWritesCoversOperationin plugin-security: that OWD opensupdateonly), and an external object has no storedcreated_by. So "public_read_write plus a hook" opens member edits, not member deletes. The rule does not warn on it (the card pinsspike_nodesilent); theos explaintext says it.Measured: which knobs make a member edit pass
One-off boot of the spike composition (engine, sqlite default datasource, security, sharing, REST) with one external object per row on a plugin driver. Rows marked "map" ran on a variant of the driver that renames the remote column the way the SQL driver applies
columnMap(remote column to local field), i.e. a driver that returns the owner under the checked name; the other rows' driver ignorescolumnMap, as every plugin driver does. Member holdsallowRead/allowEdit/allowDelete; rowsr_mine(owner or creator = member),r_theirs(= admin).The SQL-driver half was measured separately on
SqlDriver(better-sqlite3) over a remote tablerows(id, name, assignee)registered as external withcolumnMap: { assignee: 'owner_id' }: the sharing check's exact reads,find { where: { id }, fields: ['id', 'owner_id'], limit: 1 }andfindOne { where: { $and: [{ id }, { owner_id: U }] } }, both return the row withowner_id: 'u1'(with and withoutowner_iddeclared); without the map the projection returns noowner_id.sharingModel(spike_owned)owner_id)private/public_readowner_id)public_read_write(spike_node)private, map remote owner toowner_id, not declaredowner_iddeclared infieldsprivate, driver returnsowner_iditself, declaredprivate,ownership: 'none'(also unset model; also'org')created_by)private,ownership: 'none',systemFields.audit: falsecreated_by)private,ownership: 'none', map remote creator tocreated_by, not declaredcreated_bydeclaredprivate,ownership: 'none', driver returnscreated_byitself, declaredprivate,columnMapmaps onlycreated_by(owner_id still injected)owner_id)private,systemFields.audit: falseowner_id)So
ownership: 'none'is not a third way out (the neighbouringunprovisionedAnchorFixhint insystem-fields.tsoffers "opt the object out of the injection" for a different finding; here it only moves the refusal to the floor), and the rule keeps firing on it.Tests
packages/lint/src/validate-external-owner-sharing.test.ts(14 cases): the spike's two shapes throughdefineStack(spike_ownedwarns namingowner_idatobjects[1].sharingModel;spike_nodesilent; throughrunAuthoringRules('validate')it lands in advisories, never errors), then the matrix above: owner-keyed models warn;public_read_write,controlled_by_parent, read-only bindings, platform-stored objects and an unset system object are silent; a declaredowner_idsilences (with or without acolumnMapentry); acolumnMapentry alone still warns, worded for it; acreated_by-only map does not serve the owner check;ownership: 'none' | 'org' | 'business_unit'warns namingcreated_by, the audit-off variant warns, acolumnMapentry forcreated_byalone warns, a declaredcreated_bysilences.pnpm --filter @objectstack/lint testat ee143ec: 177 files / 7583 tests passed.pnpm --filter @objectstack/lint typecheck:tsc --noEmitclean,check:test-typecheckOK.owner === 'author'arm removed viascripts/ablation-replace.mjs(anchor 1 to 0, blob 1cbc9bde to 24cd3fc6), the declared-owner_idcase went red (1 failed / 13 passed), restore proven by blob == HEAD and an emptygit diff HEAD. (An earlier ablation on the first revision'scolumnMapsilence also went red; that silence was removed when the PM's measurement showedcolumnMapis not applied by the engine.)Acceptance notes
os validatesays nothing" is not literally true for the spike'sspike_owned: it already getssecurity-owd-unset(error) because it declares nosharingModel. But that rule's hint recommends'private', which on a writable external object is exactly the 403 shape; nothing named the refusal. Both now fire onspike_owned, and the new warning names the way out. Whethersecurity-owd-unset's hint should stop recommending'private'for an external object is left for the card owner; it is a wording change on a gating rule, not this card.privatewith the driver returningowner_idbut no declared field let the member READ the admin's row (GET returned 2 of 2 remaining rows; with the field declared, 1). The sharing service's phantom-anchor test readsfieldsonly, so a served-but-undeclared owner counts as unstored for reads while writes are owner-scoped. That is one more reason the rule asks for the declaration. Reported separately in the dev report; not changed here.created_by(orowner_id) and returned by the driver, but not declared, passes the checks while the rule still warns: lint cannot see what a driver returns. The hint (declare the field) is harmless there and, forowner_id, also scopes reads.os validate/os buildprint from 55 to 56; four CLI transcripts undercontent/docs/(deployment/cli.mdx,deployment/validating-metadata.mdx,getting-started/build-with-claude-code.mdx,ui/react-pages.mdx) are updated socheck:docs-transcript-driftholds. Not the virtual-objects guide.public_read_write, and "setsharingModelexplicitly" alone is not a way out (an explicit'private'is still refused); the way out is'public_read_write'.Verification
Local runs at head
ee143ecb.Derived gate families (dispatch-gates --commands, 88 commands)
node scripts/pm/dispatch-gates.mjs --commandsat ee143ec derived 88 commands; all 88 run sequentially at ee143ec, exit codes captured before any pipe;--ranwith theCOMMAND :: exit Nrecord: 88 accounted, 87 run, 1 NOT-MEASURED.check:docs-transcript-drift,check:dual-build-cjs-loads,check:doc-authoring,check:nul-bytes,check:issue-citations,check:type-check-coverage,check:type-check-debt,spec check:docs,spec check:liveness,lint check:doc-security-posture).node scripts/check-plugin-teardown-shape.mjs --self-test, exit 3 (prerequisite: its positive control is pinned to commit 621a487, which this shallow checkout cannot reach). Checker health only, not a verdict on this diff; CI's full clone runs it.check:docs-transcript-driftred (four transcripts printed 55 rules, the registry now derives 56); fixed in b0e37b9.🤖 Generated with Claude Code
https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV