Repository navigation
Commit 9229738
feat(lint): security-external-owner-unmapped warns when a writable external object refuses every member write (#27422)
<!-- claude-projects-attribution: {"github_login":"marchtian"} -->
_Requested by **田騏** · [project
thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiVy7F2atKWpo5rp3YPrqddh)_
Fixes #27332
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么:** `os validate` / `os build` / `os lint` 新增一条警告
`security-external-owner-unmapped`:可写的外部对象若用按所有者的共享模型(`private`、`public_read`,或不写),又没在
`fields` 声明所有者检查读的那一列,就提示;警告点名两条出路(改 `public_read_write` 由
`beforeUpdate` hook 判谁能改;或在 `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 `external` binding with `writable:
true`, an effective sharing model that keys writes to an owner
(`private`, `public_read`, or none on a custom object, which the runtime
treats as `private`), and no remote column serving the check that
refuses the write:
1. **Owner arm.** The object carries `owner_id` (`ownership: 'user'`,
the default) as the platform's injected, unstored anchor, not declared
in `fields`. Record sharing reads `owner_id`, gets null, and refuses.
2. **Floor arm.** The object carries no `owner_id` (`ownership: 'none' |
'org' | 'business_unit'`). Record sharing then abstains, and the
platform ownership floor (`created_by == current_user.id`) decides. It
warns when `created_by` is 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.columnMap` is applied by the
built-in SQL drivers, not by the engine, so on a plugin driver a
`columnMap` entry serves nothing. The author's vouch the rule reads is
therefore a declared `owner_id` / `created_by` field (the provenance the
runtime guards read: `resolveInjectedColumnProvenance`,
`@objectstack/spec/data`); a `columnMap` entry 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_write` plus a `beforeUpdate` hook, or
declare the field and have the driver return the remote owner under that
name (a built-in SQL driver through `external.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, why `ownership: 'none'` is no way out, deletes)
is `os 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.ts` is exported and reused instead
of copied a second time; the rule id is exported from the root barrel;
one changeset (`@objectstack/lint` patch).
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/ID` by a member holding `allowDelete` answers 403
`PERMISSION_DENIED` on the default, `private` and `public_read` models,
and the remote is asked exactly `find { where: { id }, fields: ['id',
'owner_id'], limit: 1 }` then `findOne { where: { $and: [{ id }, {
created_by: CALLER }] }, limit: 1 }`; `driver.delete` is never called.
One more fact the PR names because the warning's first way out depends
on it: on `public_read_write` the edit passes (200) but the **delete is
still refused** (403). The floor keeps deletes to `created_by` there by
design (`owdOpenWritesCoversOperation` in plugin-security: that OWD
opens `update` only), and an external object has no stored `created_by`.
So "public_read_write plus a hook" opens member edits, not member
deletes. The rule does not warn on it (the card pins `spike_node`
silent); the `os explain` text 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 ignores `columnMap`, as
every plugin driver does. Member holds
`allowRead/allowEdit/allowDelete`; rows `r_mine` (owner or creator =
member), `r_theirs` (= admin).
The SQL-driver half was measured separately on `SqlDriver`
(better-sqlite3) over a remote table `rows(id, name, assignee)`
registered as external with `columnMap: { assignee: 'owner_id' }`: the
sharing check's exact reads, `find { where: { id }, fields: ['id',
'owner_id'], limit: 1 }` and `findOne { where: { $and: [{ id }, {
owner_id: U }] } }`, both return the row with `owner_id: 'u1'` (with and
without `owner_id` declared); without the map the projection returns no
`owner_id`.
| object shape | PATCH own | PATCH other | DELETE own | warns |
|:--|:--|:--|:--|:--|
| no `sharingModel` (spike_owned) | 403 | 403 | 403 | yes (`owner_id`) |
| `private` / `public_read` | 403 | 403 | 403 | yes (`owner_id`) |
| `public_read_write` (spike_node) | 200 | 200 | 403 | no |
| `private`, map remote owner to `owner_id`, not declared | 200 | 403 |
200 | yes (columnMap wording) |
| same, plus `owner_id` declared in `fields` | 200 | 404 (read-scoped) |
200 | no |
| `private`, driver returns `owner_id` itself, declared | 200 | 404 |
200 | no |
| `private`, `ownership: 'none'` (also unset model; also `'org'`) | 403
| 403 | 403 | yes (`created_by`) |
| `private`, `ownership: 'none'`, `systemFields.audit: false` | 403 |
403 | 403 | yes (`created_by`) |
| `private`, `ownership: 'none'`, map remote creator to `created_by`,
not declared | 200 | 403 | 200 | yes (columnMap wording) |
| same, plus `created_by` declared | 200 | 403 | 200 | no |
| `private`, `ownership: 'none'`, driver returns `created_by` itself,
declared | 200 | 403 | 200 | no |
| same, not declared | 200 | 403 | 200 | yes (false positive, see notes)
|
| `private`, `columnMap` maps only `created_by` (owner_id still
injected) | 403 | 403 | 403 | yes (`owner_id`) |
| `private`, `systemFields.audit: false` | 403 | 403 | 403 | yes
(`owner_id`) |
So `ownership: 'none'` is not a third way out (the neighbouring
`unprovisionedAnchorFix` hint in `system-fields.ts` offers "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
- New `packages/lint/src/validate-external-owner-sharing.test.ts` (14
cases): the spike's two shapes through `defineStack` (`spike_owned`
warns naming `owner_id` at `objects[1].sharingModel`; `spike_node`
silent; through `runAuthoringRules('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
declared `owner_id` silences (with or without a `columnMap` entry); a
`columnMap` entry alone still warns, worded for it; a `created_by`-only
map does not serve the owner check; `ownership: 'none' | 'org' |
'business_unit'` warns naming `created_by`, the audit-off variant warns,
a `columnMap` entry for `created_by` alone warns, a declared
`created_by` silences.
- `pnpm --filter @objectstack/lint test` at ee143ec: 177 files / 7583
tests passed. `pnpm --filter @objectstack/lint typecheck`: `tsc
--noEmit` clean, `check:test-typecheck` OK.
- Ablation (one, on the rule's silence read): the `owner === 'author'`
arm removed via `scripts/ablation-replace.mjs` (anchor 1 to 0, blob
1cbc9bd to 24cd3fc6), the declared-`owner_id` case went red (1 failed /
13 passed), restore proven by blob == HEAD and an empty `git diff HEAD`.
(An earlier ablation on the first revision's `columnMap` silence also
went red; that silence was removed when the PM's measurement showed
`columnMap` is not applied by the engine.)
## Acceptance notes
- **Premise, partly corrected.** "`os validate` says nothing" is not
literally true for the spike's `spike_owned`: it already gets
`security-owd-unset` (error) because it declares no `sharingModel`. 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 on `spike_owned`, and the new warning names the way out. Whether
`security-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.
- **Read scoping with an undeclared owner.** In the measurement,
`private` with the driver returning `owner_id` but 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
reads `fields` only, 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.
- **Known false positive.** A remote table whose column is literally
named `created_by` (or `owner_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, for `owner_id`, also scopes reads.
- **Docs transcripts.** The new rule moves the author-time rule count
`os validate` / `os build` print from 55 to 56; four CLI transcripts
under `content/docs/` (`deployment/cli.mdx`,
`deployment/validating-metadata.mdx`,
`getting-started/build-with-claude-code.mdx`, `ui/react-pages.mdx`) are
updated so `check:docs-transcript-drift` holds. Not the virtual-objects
guide.
- **Guide wording.** The guide callout (PR #27403 head) and this warning
agree on edits and on the driver's job. Two sentences belong to the
guide's own card: deletes stay refused on `public_read_write`, and "set
`sharingModel` explicitly" alone is not a way out (an explicit
`'private'` is still refused); the way out is `'public_read_write'`.
- The table above came from a scratch harness that was deleted before
commit; the spike's committed step (e3) remains the durable runtime pin
for the edit half.
## Verification
Local runs at head `ee143ecb`.
<details><summary>Derived gate families (dispatch-gates --commands, 88
commands)</summary>
`node scripts/pm/dispatch-gates.mjs --commands` at ee143ec derived 88
commands; all 88 run sequentially at ee143ec, exit codes captured
before any pipe; `--ran` with the `COMMAND :: exit N` record: 88
accounted, 87 run, 1 NOT-MEASURED.
- 87 exit 0 (including `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`).
- NOT MEASURED: `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.
- First run at 4199782 found `check:docs-transcript-drift` red (four
transcripts printed 55 rules, the registry now derives 56); fixed in
b0e37b9.
</details>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 968925c commit 9229738
11 files changed
Lines changed: 500 additions & 5 deletions
File tree
- .changeset
- content/docs
- deployment
- getting-started
- ui
- packages/lint/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
758 | 758 | | |
759 | 759 | | |
760 | 760 | | |
761 | | - | |
| 761 | + | |
762 | 762 | | |
763 | 763 | | |
764 | 764 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
752 | 752 | | |
753 | 753 | | |
754 | 754 | | |
755 | | - | |
| 755 | + | |
756 | 756 | | |
757 | 757 | | |
758 | 758 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
305 | 305 | | |
306 | 306 | | |
307 | 307 | | |
308 | | - | |
| 308 | + | |
309 | 309 | | |
310 | 310 | | |
311 | 311 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
404 | 404 | | |
405 | 405 | | |
406 | 406 | | |
407 | | - | |
| 407 | + | |
408 | 408 | | |
409 | 409 | | |
410 | 410 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
137 | 137 | | |
138 | 138 | | |
139 | 139 | | |
| 140 | + | |
140 | 141 | | |
141 | 142 | | |
142 | 143 | | |
| |||
2330 | 2331 | | |
2331 | 2332 | | |
2332 | 2333 | | |
| 2334 | + | |
| 2335 | + | |
| 2336 | + | |
| 2337 | + | |
| 2338 | + | |
| 2339 | + | |
| 2340 | + | |
| 2341 | + | |
| 2342 | + | |
| 2343 | + | |
| 2344 | + | |
| 2345 | + | |
| 2346 | + | |
| 2347 | + | |
| 2348 | + | |
| 2349 | + | |
2333 | 2350 | | |
2334 | 2351 | | |
2335 | 2352 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
380 | 380 | | |
381 | 381 | | |
382 | 382 | | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
383 | 394 | | |
384 | 395 | | |
385 | 396 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
730 | 730 | | |
731 | 731 | | |
732 | 732 | | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
733 | 776 | | |
734 | 777 | | |
735 | 778 | | |
| |||
5150 | 5193 | | |
5151 | 5194 | | |
5152 | 5195 | | |
| 5196 | + | |
5153 | 5197 | | |
5154 | 5198 | | |
5155 | 5199 | | |
| |||
0 commit comments