Skip to content

fix(lint): security-owd-unset's fix line on a writable external object names security-external-owner-unmapped's ways out, not 'private' - #27639

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-6ml89f
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-6ml89f

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by 田騏 · project thread

Fixes #27472
Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么: 可写外部对象(external.writable: true)没写 sharingModel 时,security-owd-unset 的修复提示不再推荐 private,改为给出 security-external-owner-unmapped 的两条出路;os explain security-owd-unset 补了一段说明这个例外。
  • 为什么改: 旧提示对所有自定义对象一律推荐 private,而可写外部对象照做后,另一条警告又说成员每次编辑都会被拒,两条提示互相打架。
  • 风险与回滚: 只改提示文字,报错级别和报错正文不变,元数据契约不变;普通对象的提示一字未动。回滚即 revert 本 PR。
  • 先看: packages/lint/src/validate-security-posture.ts(SECURITY_OWD_UNSET 分支的 hint)、packages/lint/src/validate-external-owner-sharing.ts(导出复用的两条出路措辞与判定)、packages/lint/src/validate-external-owner-sharing.test.ts(末尾新 describe)。
  • 两分钟验证: pnpm --filter @objectstack/lint exec vitest run src/validate-external-owner-sharing.test.ts,预期全绿,其中 security-owd-unset's fix line on a writable external object 五条通过。
  • 需要你做的: 无,席位落地。

What changes

  • security-owd-unset fix line (validate-security-posture.ts). For a custom object with no sharingModel that security-external-owner-unmapped would judge, the hint now names that rule's two ways out instead of recommending 'private'. Those objects are a writable binding (external.writable: true), not on a datasource this stack declares with external.allowWrites: false, and declaring no ownership column a remote row serves (owner_id, or created_by on an object without one). The new hint reads, for spike_owned:
    declare sharingModel: 'public_read_write' and decide who may edit in a beforeUpdate hook, or declare owner_id in fields and have the driver return the remote owner under that name (a built-in SQL driver does it through external.columnMap; a plugin driver must return it itself) and declare sharingModel: 'private' or 'public_read'; 'controlled_by_parent' for master-detail children
    Severity (error), the verdict line and path are unchanged. Every other object keeps today's hint byte for byte.
  • One phrasing, not three (validate-external-owner-sharing.ts). PUBLIC_READ_WRITE_WAY_OUT and the declaration clause (declareOwnershipColumnWayOut(column)) are now exported from the rule that owns them. The judgement "does this object have an ownership gap" moves into externalOwnershipGap(obj, readOnlyDatasources), and the datasource set into readOnlyExternalDatasources(stack). Both rules call the same predicate, so the hint swaps on exactly the objects the warning fires on. These are module exports, not barrel exports: the package's public surface is unchanged. The external rule's own findings are byte-identical, and its existing whole-hint pins stay green.
  • os explain security-owd-unset (rule-explanations.ts). A new paragraph explains the writable-external exception to the 'private' recommendation, names the two ways out and points at os explain security-external-owner-unmapped.
  • Changeset: @objectstack/lint patch.

Tests

  • validate-external-owner-sharing.test.ts adds the describe block security-owd-unset's fix line on a writable external object:
    • spike_owned (the spike stack, verbatim) gets the external hint (both shared clauses, no recommended), with severity, path and message unchanged;
    • a managed custom object (leave_request) keeps the old hint, pinned whole;
    • an ownership: 'none' external object is told to declare created_by;
    • a read-only binding, a declared owner_id and an allowWrites: false datasource keep the old hint;
    • either way out, once taken (public_read_write, or private plus a declared owner_id), clears both rules.
  • validate-security-posture.test.ts: the explanation pin also asserts the external paragraph. The receiver meta-test (validate-expressions / validate-security-posture 也有同形的 spec 不声明键的 ?? 别名读法(#5009 建议 3 的核对结果) #5017) excuses ownershipGap, which is the helper's own return value and not metadata.
  • Ablation: with the fix committed, scripts/ablation-replace.mjs replaced const ownershipGap = externalOwnershipGap(obj, readOnlyDatasources); with undefined (anchor 1 → 0, blob ce9dd0f8e857 → f178c04309b6). Two of the new pins went red (spike_owned …, names created_by …), 24 stayed green. The tool restored the file (blob == HEAD, git diff HEAD empty). The test imports src/ by relative path, so no dist/ rebuild was involved.

Acceptance notes

  • The hint swaps only where the warning fires, which is narrower than the card's literal "external.writable: true". An external object that already declares owner_id (or created_by on an object without owner_id), or that sits on an allowWrites: false datasource, keeps the 'private' recommendation. There security-external-owner-unmapped is silent and 'private' is not the refused shape: the owner edits, and since fix(plugin-sharing)!: an unproven owner anchor on a federated object narrows the read and bulk-write scopes, never leaves them unscoped #27476 reads are scoped by the owner it proves. This follows the card's first clause ("names the ways out security-external-owner-unmapped names"). Where that rule names none, there is nothing to name.
  • The guide (content/docs/data-modeling/virtual-objects.mdx, the "Choose the sharing model on purpose" callout) still reads true and is not edited.
  • packages/cli/test/explain-rule-id.test.ts pins the managed-object hint, which is unchanged. It is declared to CI: the CLI's dependency closure is not built in this container (the file fails to load on @objectstack/types resolution, before any assertion runs).

Verification

Local runs (head 4e9b79c)

The tests ran on the code tree of c4b4bd65. The head 4e9b79ca adds only the changeset, so its code is the same. The gate commands ran at 4e9b79ca.

  • pnpm --filter '@objectstack/lint^...' build under os-verify-lock.sh: VERDICT command-exit 0.
  • pnpm --filter @objectstack/lint typecheck && pnpm --filter @objectstack/lint test under the lock: VERDICT command-exit 0; Test Files 177 passed (177), Tests 7604 passed | 5 skipped (7609). The typecheck ran tsc --noEmit and check:test-typecheck.
  • Targeted: vitest run src/validate-external-owner-sharing.test.ts src/validate-security-posture.test.ts src/rule-explanations.test.ts src/validate-security-posture.runtime-surface.test.ts: Test Files 4 passed (4), Tests 228 passed (228).
  • Ablation (scripts/ablation-replace.mjs, see Tests): VERDICT command-exit 1, Tests 2 failed | 24 passed (26), restored blob == HEAD (ce9dd0f8e857), git diff HEAD empty.
  • node scripts/pm/dispatch-gates.mjs --commands: 58 commands derived; all 58 were run with their exit codes recorded. --ran: "58 derived famil(ies) accounted for — 55 run, 3 NOT-MEASURED (3 DERIVED from a recorded exit 3)". 55 exited 0, including check:docs-transcript-drift ("4 declared transcript value(s) across 471 page(s) … equal what the registry derives today"), check:doc-authoring, check:nul-bytes, check:cross-package-test-inputs and check:type-check-debt.
  • NOT MEASURED, exit 3 (prerequisite), left to CI:
    • node scripts/check-plugin-teardown-shape.mjs --self-test: a shallow clone cannot reach its pinned positive-control commit;
    • pnpm check:dual-build-cjs-loads and pnpm check:lean-entry-closure: both read built dist/ of packages outside this closure.
  • NOT MEASURED, declared to CI: packages/cli/test/explain-rule-id.test.ts. The CLI closure is not built here, so the file fails to load (@objectstack/types entry unresolved) before any assertion runs. It pins the managed-object hint, which this diff leaves unchanged.

🤖 Generated with Claude Code

https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV

…t names security-external-owner-unmapped's ways out

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6.

⛔ 11 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 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 — 4 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 efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 87e015dccc0f2c38c594c63e59e5f60369bf0181 — the merge of head 78e68ad43b69cdfc71665680aa652e25e3635ccc into base efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6, 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 87e015dccc0f2c38c594c63e59e5f60369bf0181 && git checkout 87e015dccc0f2c38c594c63e59e5f60369bf0181
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6 78e68ad43b69cdfc71665680aa652e25e3635ccc && git checkout -B drift-repro efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6 && git merge --no-ff 78e68ad43b69cdfc71665680aa652e25e3635ccc

node scripts/docs-audit/affected-docs.mjs --json efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6

⚠️ 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 efa2915e4c2dc1d5bd09bedbd4eef9bc31cb9fe6 → 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

epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV
REWORK round 1 · PR #27639 @ 4e9b79ca · 2026-10-29T16:01Z

The seat review and an independent full Fable review agree.

Must fix

  1. The hint still recommends 'private' where 'private' reads no rows. On security-owd-unset's side, externalOwnershipGap exempts an object on a datasource declared external.allowWrites: false, which keeps "'private' … recommended". On main since fix(plugin-sharing)!: an unproven owner anchor on a federated object narrows the read and bulk-write scopes, never leaves them unscoped #27476, buildReadFilter answers deny-all on private whenever the owner anchor is a phantom (hasPhantomOwnerAnchor: external != null plus owner provenance). Writability and allowWrites play no part, so a member following the hint reads zero rows.
    • The exemption is also inconsistent across doors. The posture rule runs at the publish gate, whose RuntimeStackContext carries no datasources, so os lint and the 422 door disagree on that shape.
    • Fix: keep the datasource exemption in security-external-owner-unmapped only, where it is about who refuses a write. security-owd-unset judges the read-side fact: an external object with no provable owner anchor (no declared owner field, no columnMap onto it) never gets 'private' recommended.
      • Writable binding: the external rule's two ways out, as now.
      • Read-only binding (writable not true, or allowWrites: false): recommend 'public_read', the guide's own advice (virtual-objects.mdx, the read-only example).
    • Flip the allowWrites: false test row, add a read-only-binding row, and rewrite the os explain sentence (rule-explanations.ts ~:124-125) that says such an object "keeps the usual fix line".
  2. Stale header at validate-external-owner-sharing.ts:41-43: it still says the injected owner "leaves reads unscoped". Since sharing: a private external object whose driver serves owner_id without declaring it in fields gives every member every row — hasPhantomOwnerAnchor reads fields only, so the read filter goes unscoped (epic #27261, found by #27332's dev) #27425 those reads are refused. Correct it to match.

Nit (optional)

  • The mapped case is worded differently in the two rules. Align it if it is cheap.

Holds

The trigger is otherwise exact. The external rule's findings are byte-identical. The managed hint is byte-identical, and the CLI pin uses a managed object. The PLUMBING exemption is justified. Changeset: patch · Clause-②: no · Metadata-protocol: none. The tests make real assertions. Card #27472's Task reads "an object with external.writable: true"; the read-only-binding row widens it by one row on the same fact, as the epic PM's scope call.

Same branch, same claim. The surface stays packages/lint/src/** and .changeset/**.

…al object with no provable owner; a read-only binding is told 'public_read'

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV
ACCEPT · PR #27639 @ 78e68ad43b69cdfc71665680aa652e25e3635ccc · 2026-10-29T16:36Z

维护者速读

席位意见:ACCEPT。独立 Fable 完整复核(4e9b79ca)REWORK 两项,本轮 78e68ad4 由同一复核者 delta 复核 ACCEPT-with-nits,两项 must-fix 与可选 nit 均已关闭。

席位核对

  1. Shape: +240/−28, all under packages/lint/src/** and .changeset/**. The reviewer confirmed hasUnprovableOwnerAnchor agrees with plugin-sharing's hasPhantomOwnerAnchor on main for every shape (absent owner, declared owner field, columnMap onto owner_id, post-injection snapshot). It also confirmed 'public_read' adds no read filter, so the read-only line is true. The external rule's mapped hint is byte-identical to before.
  2. Premise measured:
    • the dev's ablation of the read-only arm turns exactly one pin red;
    • the reviewer's own run of the 4 targeted files at 78e68ad4 passes 230;
    • the dev's full lint suite passes 7606 (5 skipped);
    • 58 derived gates ran, 3 NOT-MEASURED left to CI (shallow clone, no full build).
  3. Gate check-runs on 78e68ad4: 48 runs: 38 success, 10 skipped, 0 failed or pending. mergeable_state: clean.
  4. Accepted deviations:
    • the read-only-binding row widens the card's Task by one row on the same fact (epic PM's scope call, recorded in the REWORK);
    • the publish gate carries no datasources, so a writable binding on an allowWrites: false datasource gets the two-ways-out line at the 422 door and 'public_read' from os lint. Neither recommends 'private'; this is stated in the code comment and the changeset.
  5. Out of scope, carried by the seat: a read-only binding whose columnMap targets owner_id on a plugin driver keeps the 'private' hint, while the runtime notes that predicate may match no row if the driver ignores the map. This is the same columnMap diagnostic gap already recorded on the epic; it rides there.
  6. Landing: not governed (check-governed-merges --pr 27639: 268 lines ≤ 3000). No contract face is touched (Clause-②: no, no published schema, not governed), so CI plus this read suffices. The domain:spec independent Fable review is on record above. Landed by the seat through the relay.

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

2 participants