From 3a9e77bf93e4b40930005a56f966659276d97bfe Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:18:55 +0000 Subject: [PATCH 1/5] fix(lint): security-owd-unset's fix line on a writable external object names security-external-owner-unmapped's ways out Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV --- packages/lint/src/rule-explanations.ts | 9 +++ .../validate-external-owner-sharing.test.ts | 66 ++++++++++++++++++ .../src/validate-external-owner-sharing.ts | 69 ++++++++++++++----- .../src/validate-security-posture.test.ts | 3 + .../lint/src/validate-security-posture.ts | 22 +++++- 5 files changed, 148 insertions(+), 21 deletions(-) diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index f202847c348..80f651ae74f 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -114,6 +114,15 @@ const SECURITY_OWD_UNSET_EXPLANATION: RuleExplanation = { 'default); \'public_read\' — everyone reads, the owner writes; \'public_read_write\' — everyone ' + 'reads and writes; \'controlled_by_parent\' — access derives from the master record (a ' + 'master-detail child).', + 'A writable external object (`external.writable: true`) is the exception to that recommendation. ' + + 'The remote row carries no `owner_id` or `created_by` the platform can read, so on \'private\' every ' + + 'member edit is refused, and where the object carries `owner_id` a member reads no row at all ' + + '(`os explain security-external-owner-unmapped`). There the fix line names that rule\'s two ways out ' + + 'instead: \'public_read_write\' with a `beforeUpdate` hook that decides who may edit, or the ownership ' + + 'column (`owner_id`, or `created_by` on an object without one) declared in `fields` and returned by ' + + 'the driver, after which \'private\' or \'public_read\' admits edits by the user that column names. ' + + 'An external object that already declares that column, or whose datasource lets no write through, ' + + 'keeps the usual fix line.', 'System objects (`isSystem: true`, or a name starting with `sys_`) are not judged: the platform ' + 'owns their posture.', ], diff --git a/packages/lint/src/validate-external-owner-sharing.test.ts b/packages/lint/src/validate-external-owner-sharing.test.ts index fd188848dce..b36fe50c16d 100644 --- a/packages/lint/src/validate-external-owner-sharing.test.ts +++ b/packages/lint/src/validate-external-owner-sharing.test.ts @@ -4,9 +4,12 @@ import { describe, expect, it } from 'vitest'; import { defineStack } from '@objectstack/spec'; import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; import { + PUBLIC_READ_WRITE_WAY_OUT, SECURITY_EXTERNAL_OWNER_UNMAPPED, + declareOwnershipColumnWayOut, validateExternalOwnerSharing, } from './validate-external-owner-sharing.js'; +import { SECURITY_OWD_UNSET, validateSecurityPosture } from './validate-security-posture.js'; /** * The two external objects of `packages/runtime/src/plugin-driver-external-object.spike.test.ts`, @@ -220,3 +223,66 @@ describe('security-external-owner-unmapped — the fix line', () => { } }); }); + +// [#27472] `security-owd-unset` refuses a custom object with no sharingModel, and its fix line used +// to recommend 'private' everywhere: on a writable external object that is the shape this rule warns +// refuses every member edit. Its fix line for the objects this rule judges is built from this +// rule's own two ways out; every other object keeps the 'private' recommendation. +describe("security-owd-unset's fix line on a writable external object", () => { + const owdUnsetHint = (stack: Record, where: string) => { + const findings = validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_OWD_UNSET && f.where === where); + expect(findings, where).toHaveLength(1); + return findings[0]!.hint ?? ''; + }; + const OLD_HINT = + "declare sharingModel: 'private' (owner + shares; recommended), 'public_read', " + + "'public_read_write', or 'controlled_by_parent' (master-detail children)"; + + it("spike_owned gets this rule's ways out and no 'private' recommendation", () => { + const hint = owdUnsetHint(SPIKE_STACK, 'object "spike_owned"'); + expect(hint).toContain(PUBLIC_READ_WRITE_WAY_OUT); + expect(hint).toContain(declareOwnershipColumnWayOut('owner_id')); + expect(hint).not.toContain('recommended'); + expect(hint).not.toContain('\n'); + // Severity and verdict are the rule's own and do not move. + const [finding] = validateSecurityPosture(SPIKE_STACK).filter((f) => f.where === 'object "spike_owned"'); + expect(finding).toMatchObject({ severity: 'error', path: 'objects[1].sharingModel' }); + expect(finding!.message).toBe( + "custom object declares no sharingModel (OWD); the runtime falls back to 'private', " + + 'but the baseline must be an authored decision', + ); + }); + + it('a managed custom object keeps the private recommendation', () => { + expect(owdUnsetHint({ objects: [{ name: 'leave_request', label: 'Leave Request' }] }, 'object "leave_request"')).toBe( + OLD_HINT, + ); + }); + + it('names created_by on an external object without owner_id', () => { + const hint = owdUnsetHint({ objects: [ext({ ownership: 'none' })] }, 'object "remote_row"'); + expect(hint).toContain(declareOwnershipColumnWayOut('created_by')); + expect(hint).not.toContain('recommended'); + }); + + it('keeps the private recommendation where this rule judges nothing', () => { + const ownerField = { ...NAME_FIELD, owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner' } }; + const readOnlyDatasource = { name: 'remote', schemaMode: 'external', external: { allowWrites: false } }; + for (const [label, stack] of [ + ['read-only binding', { objects: [ext({}, { writable: false })] }], + ['declared owner_id', { objects: [ext({ fields: ownerField })] }], + ['allowWrites: false', { datasources: [readOnlyDatasource], objects: [ext()] }], + ] as const) { + expect(owdUnsetHint(stack, 'object "remote_row"'), label).toBe(OLD_HINT); + } + }); + + it('either way out it names, once taken, clears this rule too', () => { + const ownerField = { ...NAME_FIELD, owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner' } }; + for (const obj of [ext({ sharingModel: 'public_read_write' }), ext({ sharingModel: 'private', fields: ownerField })]) { + const stack = { objects: [obj] }; + expect(validateSecurityPosture(stack).map((f) => f.rule)).toEqual([]); + expect(validateExternalOwnerSharing(stack)).toEqual([]); + } + }); +}); diff --git a/packages/lint/src/validate-external-owner-sharing.ts b/packages/lint/src/validate-external-owner-sharing.ts index 3fb0ab1680c..a33881ae688 100644 --- a/packages/lint/src/validate-external-owner-sharing.ts +++ b/packages/lint/src/validate-external-owner-sharing.ts @@ -99,9 +99,23 @@ type AnyRec = Record; const OWNER_COLUMN = 'owner_id'; const CREATOR_COLUMN = 'created_by'; -const PUBLIC_READ_WRITE_WAY_OUT = +/** + * The two ways out this rule names, as fix-line clauses. `security-owd-unset` + * words its fix line for the same objects from these, so an author following + * either rule's advice lands on a shape the other does not refuse. + */ +export const PUBLIC_READ_WRITE_WAY_OUT = "declare sharingModel: 'public_read_write' and decide who may edit in a beforeUpdate hook"; +/** The declaration way out, for the column {@link externalOwnershipGap} names. */ +export function declareOwnershipColumnWayOut(column: string): string { + const role = column === OWNER_COLUMN ? 'owner' : 'creator'; + return ( + `declare ${column} in fields and have the driver return the remote ${role} under that name ` + + '(a built-in SQL driver does it through external.columnMap; a plugin driver must return it itself)' + ); +} + /** Does `external.columnMap` (remote column → local field) map some remote column to `field`? */ function columnMapTargets(external: AnyRec, field: string): boolean { const map = external.columnMap; @@ -129,14 +143,13 @@ function undeclaredOwnershipColumn(obj: AnyRec): { column: string; injected: boo * declares no ownership column a remote row serves. Returns the findings * (empty = clean); the CLI folds them in as advisory warnings. */ -export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharingFinding[] { - const out: ExternalOwnerSharingFinding[] = []; - if (!stack || typeof stack !== 'object') return out; - const objects = recordsOf(stack.objects); - // ADR-0015's datasource half of the double write opt-in: a datasource this - // stack declares with `external.allowWrites: false` (also the parsed - // default) lets no write through, so no ownership check is what refuses. - const readOnlyDatasources = new Set( +/** + * ADR-0015's datasource half of the double write opt-in: the datasources this + * stack declares with `external.allowWrites: false` (also the parsed default) + * let no write through, so no ownership check is what refuses. + */ +export function readOnlyExternalDatasources(stack: AnyRec): ReadonlySet { + return new Set( recordsOf(stack.datasources) .filter((ds) => { const ext = ds.external; @@ -144,24 +157,46 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin }) .map((ds) => ds.name), ); +} + +/** + * The ownership column whose missing storage refuses every member write on + * this object under an owner-keyed sharing model — or `undefined` when the + * object is not a writable external binding, sits on a datasource that lets + * no write through, or declares the column the check reads. The sharing model + * itself is the caller's to judge. + */ +export function externalOwnershipGap( + obj: AnyRec, + readOnlyDatasources: ReadonlySet, +): { column: string; injected: boolean } | undefined { + const external = obj.external; + if (!external || typeof external !== 'object' || Array.isArray(external)) return undefined; + if ((external as AnyRec).writable !== true) return undefined; + if (readOnlyDatasources.has(obj.datasource)) return undefined; + return undeclaredOwnershipColumn(obj); +} + +export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharingFinding[] { + const out: ExternalOwnerSharingFinding[] = []; + if (!stack || typeof stack !== 'object') return out; + const objects = recordsOf(stack.objects); + const readOnlyDatasources = readOnlyExternalDatasources(stack); objects.forEach((obj, i) => { - const external = obj.external; - if (!external || typeof external !== 'object' || Array.isArray(external)) return; - if ((external as AnyRec).writable !== true) return; - if (readOnlyDatasources.has(obj.datasource)) return; if (effectiveSharingModelOf(obj) === 'public') return; - const undeclared = undeclaredOwnershipColumn(obj); + const undeclared = externalOwnershipGap(obj, readOnlyDatasources); if (!undeclared) return; const { column } = undeclared; + const external = obj.external as AnyRec; const name = typeof obj.name === 'string' ? obj.name : `(object ${i})`; const model = typeof obj.sharingModel === 'string' ? `'${obj.sharingModel}'` : "'private' (the default)"; const check = column === OWNER_COLUMN ? 'record sharing reads owner_id' : 'with no owner_id, the ownership floor reads created_by == the caller'; - const mapped = columnMapTargets(external as AnyRec, column); + const mapped = columnMapTargets(external, column); const role = column === OWNER_COLUMN ? 'owner' : 'creator'; // [#27425] On `private`, record sharing also scopes READS by owner_id, and // an owner it cannot read admits no row: the edit's pre-image read finds @@ -170,9 +205,7 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin column === OWNER_COLUMN && effectiveSharingModelOf(obj) === 'private' ? "(404: the member's read scope admits no row)" : '(403)'; - const declareWayOut = - `declare ${column} in fields and have the driver return the remote ${role} under that name ` + - '(a built-in SQL driver does it through external.columnMap; a plugin driver must return it itself)'; + const declareWayOut = declareOwnershipColumnWayOut(column); out.push({ severity: 'warning', diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index eb573fa4809..aa566bb7e81 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -145,6 +145,9 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { const text = explanation!.paragraphs.join('\n'); expect(text).toContain('ADR-0090 D1'); expect(text).toContain("read and edit every other user's records"); + // [#27472] The writable-external exception to the 'private' recommendation, and the rule it defers to. + expect(text).toContain('external.writable: true'); + expect(text).toContain('os explain security-external-owner-unmapped'); expect(finding.message).not.toContain('incident'); }); diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index 7122b8ad13d..ce9dd0f8e85 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -125,6 +125,12 @@ import { controlledByParentOwnerMayEdit, isControlledByParentMasterField, refere import { describeAnchorForbiddenBits, type AnchorBindingContext } from '@objectstack/spec/security'; import { indexObjectGraph, nearestName, packagesOf, recordsOf, type ObjectGraph } from './object-graph.js'; import { injectedColumnsFor } from './system-fields.js'; +import { + PUBLIC_READ_WRITE_WAY_OUT, + declareOwnershipColumnWayOut, + externalOwnershipGap, + readOnlyExternalDatasources, +} from './validate-external-owner-sharing.js'; export const SECURITY_OWD_UNSET = 'security-owd-unset'; export const SECURITY_OWD_ALIAS = 'security-owd-alias'; @@ -794,6 +800,7 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } declaredCapabilities.length > 0 ? { declaredCapabilities } : undefined; // ── D1/D4: per-object OWD posture ─────────────────────────────────── + const readOnlyDatasources = readOnlyExternalDatasources(stack); for (let i = 0; i < objects.length; i++) { const obj = objects[i]; if (!obj || typeof obj !== 'object') continue; @@ -803,6 +810,13 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } if (!isSystemObject(obj)) { if (owd == null) { + // [#27472] On a writable external object whose ownership column no + // remote row serves, 'private' is the shape that refuses every member + // edit (`security-external-owner-unmapped`), so the fix line names that + // rule's ways out instead of recommending it. Every other object — + // an external one that declares its owner column included — keeps the + // 'private' recommendation. + const ownershipGap = externalOwnershipGap(obj, readOnlyDatasources); findings.push({ severity: 'error', rule: SECURITY_OWD_UNSET, @@ -815,9 +829,11 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } message: `custom object declares no sharingModel (OWD); the runtime falls back to 'private', ` + `but the baseline must be an authored decision`, - hint: - `declare sharingModel: 'private' (owner + shares; recommended), 'public_read', ` + - `'public_read_write', or 'controlled_by_parent' (master-detail children)`, + hint: ownershipGap + ? `${PUBLIC_READ_WRITE_WAY_OUT}, or ${declareOwnershipColumnWayOut(ownershipGap.column)} ` + + `and declare sharingModel: 'private' or 'public_read'; 'controlled_by_parent' for master-detail children` + : `declare sharingModel: 'private' (owner + shares; recommended), 'public_read', ` + + `'public_read_write', or 'controlled_by_parent' (master-detail children)`, }); } else if (typeof owd === 'string' && OWD_ALIAS_FIX[owd]) { // Reachable ONLY through the unparsed doors (`os lint` on a raw From c4b4bd650ba754784f9f9b30475a42620af70f6a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:21:24 +0000 Subject: [PATCH 2/5] test(lint): excuse the ownership-gap helper's answer in the receiver meta-test Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV --- packages/lint/src/validate-security-posture.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index aa566bb7e81..0d7853b3421 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -1795,6 +1795,8 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te 'cbpDetails', // #26591: the resolving details by name — JS Map methods. 'policies', // one set's `rowLevelSecurity` records — `.length`, a JS property. 'predicate', // a policy's `using` / `check` text, a string — `.trim`. + // #27472: `externalOwnershipGap`'s answer — `.column`, a field of that helper's own return value. + 'ownershipGap', ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); }); From 4e9b79ca9f728f74a183aae71f81bd6e7c88f42b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:23:25 +0000 Subject: [PATCH 3/5] chore(changeset): security-owd-unset external fix line Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV --- .changeset/27472-owd-unset-external-hint.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/27472-owd-unset-external-hint.md diff --git a/.changeset/27472-owd-unset-external-hint.md b/.changeset/27472-owd-unset-external-hint.md new file mode 100644 index 00000000000..531897cce38 --- /dev/null +++ b/.changeset/27472-owd-unset-external-hint.md @@ -0,0 +1,16 @@ +--- +'@objectstack/lint': patch +--- + +fix(lint): `security-owd-unset`'s fix line on a writable external object names the ways out `security-external-owner-unmapped` names, instead of recommending `'private'` + +Clause-②: no +Metadata-protocol: none + +`security-owd-unset` refuses a custom object with no `sharingModel`, and its fix line recommended `sharingModel: 'private'` for every such object. On a writable external object (`external.writable: true`) whose ownership column no remote row serves, `'private'` is the shape `security-external-owner-unmapped` warns refuses every member edit: the author followed one rule's advice into the other's warning. + +- **Now.** For exactly the objects `security-external-owner-unmapped` judges (a writable binding, on a datasource this stack does not declare with `external.allowWrites: false`, with no declared `owner_id`, or `created_by` on an object without `owner_id`), the fix line reads: declare `sharingModel: 'public_read_write'` and decide who may edit in a `beforeUpdate` hook, or declare the ownership column in `fields` and have the driver return the remote owner under that name, and declare `'private'` or `'public_read'`; `'controlled_by_parent'` for master-detail children. Both clauses are that rule's own wording. +- **Unchanged.** The severity (`error`) and the verdict line. Every other custom object, and an external object that already declares its ownership column, keeps the `'private'` recommendation. +- `os explain security-owd-unset` explains the writable-external exception and points at `os explain security-external-owner-unmapped`. + +The same fix line reaches the runtime publish gate's 422 issue for an `object` write. From e0b15c2521b0238d8afa5ae9956dc432832239b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:04:51 +0000 Subject: [PATCH 4/5] fix(lint): security-owd-unset never recommends 'private' on an external object with no provable owner; a read-only binding is told 'public_read' Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV --- packages/lint/src/rule-explanations.ts | 11 ++-- .../validate-external-owner-sharing.test.ts | 31 ++++++++-- .../src/validate-external-owner-sharing.ts | 61 ++++++++++++++----- .../src/validate-security-posture.test.ts | 1 + .../lint/src/validate-security-posture.ts | 28 ++++++--- 5 files changed, 99 insertions(+), 33 deletions(-) diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 80f651ae74f..c4c88a2f6c3 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -114,15 +114,18 @@ const SECURITY_OWD_UNSET_EXPLANATION: RuleExplanation = { 'default); \'public_read\' — everyone reads, the owner writes; \'public_read_write\' — everyone ' + 'reads and writes; \'controlled_by_parent\' — access derives from the master record (a ' + 'master-detail child).', - 'A writable external object (`external.writable: true`) is the exception to that recommendation. ' + - 'The remote row carries no `owner_id` or `created_by` the platform can read, so on \'private\' every ' + + 'An external object with no provable owner is the exception to that recommendation. On a writable ' + + 'binding (`external.writable: true`) the remote row carries no `owner_id` or `created_by` the platform can read, so on \'private\' every ' + 'member edit is refused, and where the object carries `owner_id` a member reads no row at all ' + '(`os explain security-external-owner-unmapped`). There the fix line names that rule\'s two ways out ' + 'instead: \'public_read_write\' with a `beforeUpdate` hook that decides who may edit, or the ownership ' + 'column (`owner_id`, or `created_by` on an object without one) declared in `fields` and returned by ' + 'the driver, after which \'private\' or \'public_read\' admits edits by the user that column names. ' + - 'An external object that already declares that column, or whose datasource lets no write through, ' + - 'keeps the usual fix line.', + 'A read-only external binding (`external.writable` not `true`, or a datasource declared with ' + + '`external.allowWrites: false`) whose `owner_id` no declared field or `external.columnMap` entry names ' + + 'is told to declare \'public_read\' instead: no member writes it, and on \'private\' a member reads none ' + + 'of its rows. An external object whose owner is provable (a declared field, or on a read-only binding ' + + 'a `columnMap` entry onto `owner_id`) keeps the usual fix line.', 'System objects (`isSystem: true`, or a name starting with `sys_`) are not judged: the platform ' + 'owns their posture.', ], diff --git a/packages/lint/src/validate-external-owner-sharing.test.ts b/packages/lint/src/validate-external-owner-sharing.test.ts index b36fe50c16d..10bcce6fbd9 100644 --- a/packages/lint/src/validate-external-owner-sharing.test.ts +++ b/packages/lint/src/validate-external-owner-sharing.test.ts @@ -5,6 +5,7 @@ import { defineStack } from '@objectstack/spec'; import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; import { PUBLIC_READ_WRITE_WAY_OUT, + READ_ONLY_EXTERNAL_WAY_OUT, SECURITY_EXTERNAL_OWNER_UNMAPPED, declareOwnershipColumnWayOut, validateExternalOwnerSharing, @@ -265,13 +266,35 @@ describe("security-owd-unset's fix line on a writable external object", () => { expect(hint).not.toContain('recommended'); }); - it('keeps the private recommendation where this rule judges nothing', () => { - const ownerField = { ...NAME_FIELD, owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner' } }; + it('words the mapped case as this rule does: only the declaration is missing', () => { + const hint = owdUnsetHint({ objects: [ext({}, { columnMap: { assignee: 'owner_id' } })] }, 'object "remote_row"'); + expect(hint).toContain(declareOwnershipColumnWayOut('owner_id', true)); + expect(hint).toContain(PUBLIC_READ_WRITE_WAY_OUT); + expect(hint).not.toContain('recommended'); + }); + + // [#27425] On 'private', an external object whose owner_id is the platform's unprovable anchor + // reads no row for a member, whatever its writability: a read-only binding is told 'public_read'. + it("recommends 'public_read' on a read-only binding with an unprovable owner", () => { const readOnlyDatasource = { name: 'remote', schemaMode: 'external', external: { allowWrites: false } }; + const { writable: _writable, ...noWritable } = ext().external; for (const [label, stack] of [ - ['read-only binding', { objects: [ext({}, { writable: false })] }], - ['declared owner_id', { objects: [ext({ fields: ownerField })] }], + ['writable: false', { objects: [ext({}, { writable: false })] }], + ['writable unset', { objects: [{ ...ext(), external: noWritable }] }], ['allowWrites: false', { datasources: [readOnlyDatasource], objects: [ext()] }], + ] as const) { + const hint = owdUnsetHint(stack, 'object "remote_row"'); + expect(hint, label).toBe(READ_ONLY_EXTERNAL_WAY_OUT); + expect(hint, label).not.toContain('recommended'); + } + }); + + it('keeps the private recommendation where the owner is provable', () => { + const ownerField = { ...NAME_FIELD, owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner' } }; + for (const [label, stack] of [ + ['declared owner_id, writable', { objects: [ext({ fields: ownerField })] }], + ['declared owner_id, read-only', { objects: [ext({ fields: ownerField }, { writable: false })] }], + ['columnMap onto owner_id, read-only', { objects: [ext({}, { writable: false, columnMap: { assignee: 'owner_id' } })] }], ] as const) { expect(owdUnsetHint(stack, 'object "remote_row"'), label).toBe(OLD_HINT); } diff --git a/packages/lint/src/validate-external-owner-sharing.ts b/packages/lint/src/validate-external-owner-sharing.ts index a33881ae688..f0a7b40acc6 100644 --- a/packages/lint/src/validate-external-owner-sharing.ts +++ b/packages/lint/src/validate-external-owner-sharing.ts @@ -37,9 +37,11 @@ * column to `owner_id` changes nothing, and the driver must return the field * itself. So the author's vouch this rule reads is a DECLARED field — the same * provenance the runtime guards read (`resolveInjectedColumnProvenance`, - * `@objectstack/spec/data`) — never a `columnMap` entry alone. A declared - * `owner_id` also scopes reads by owner; the platform's own injected one does - * not (plugin-sharing treats it as unstored and leaves reads unscoped). + * `@objectstack/spec/data`) — never a `columnMap` entry alone. Reads follow + * the same anchor: a declared `owner_id` scopes them by owner, while the + * platform's own injected one, with no `columnMap` entry naming it, proves no + * owner, so on an owner-scoped model a member's read scope admits no row + * (plugin-sharing `hasPhantomOwnerAnchor`, #27425) and an edit answers 404. * * ## The two arms * @@ -107,15 +109,29 @@ const CREATOR_COLUMN = 'created_by'; export const PUBLIC_READ_WRITE_WAY_OUT = "declare sharingModel: 'public_read_write' and decide who may edit in a beforeUpdate hook"; -/** The declaration way out, for the column {@link externalOwnershipGap} names. */ -export function declareOwnershipColumnWayOut(column: string): string { +/** + * The declaration way out, for the column {@link externalOwnershipGap} names. + * `mapped`: an `external.columnMap` entry already targets the column, so what + * is missing is only the declared field. + */ +export function declareOwnershipColumnWayOut(column: string, mapped = false): string { const role = column === OWNER_COLUMN ? 'owner' : 'creator'; + if (mapped) return `declare ${column} in fields (a plugin driver must return the remote ${role} under that name itself)`; return ( `declare ${column} in fields and have the driver return the remote ${role} under that name ` + '(a built-in SQL driver does it through external.columnMap; a plugin driver must return it itself)' ); } +/** + * The fix-line clause `security-owd-unset` gives a read-only external binding + * whose owner anchor is unprovable ({@link hasUnprovableOwnerAnchor}): no + * member writes it, and on 'private' a member reads none of its rows. + */ +export const READ_ONLY_EXTERNAL_WAY_OUT = + "declare sharingModel: 'public_read' (no member writes a read-only binding, and on 'private' a member " + + 'reads no row: no declared field or external.columnMap entry names the remote owner_id)'; + /** Does `external.columnMap` (remote column → local field) map some remote column to `field`? */ function columnMapTargets(external: AnyRec, field: string): boolean { const map = external.columnMap; @@ -163,18 +179,35 @@ export function readOnlyExternalDatasources(stack: AnyRec): ReadonlySet * The ownership column whose missing storage refuses every member write on * this object under an owner-keyed sharing model — or `undefined` when the * object is not a writable external binding, sits on a datasource that lets - * no write through, or declares the column the check reads. The sharing model - * itself is the caller's to judge. + * no write through, or declares the column the check reads. `mapped`: an + * `external.columnMap` entry targets the column, so only the declaration is + * missing. The sharing model itself is the caller's to judge. */ export function externalOwnershipGap( obj: AnyRec, readOnlyDatasources: ReadonlySet, -): { column: string; injected: boolean } | undefined { +): { column: string; injected: boolean; mapped: boolean } | undefined { const external = obj.external; if (!external || typeof external !== 'object' || Array.isArray(external)) return undefined; if ((external as AnyRec).writable !== true) return undefined; if (readOnlyDatasources.has(obj.datasource)) return undefined; - return undeclaredOwnershipColumn(obj); + const undeclared = undeclaredOwnershipColumn(obj); + if (!undeclared) return undefined; + return { ...undeclared, mapped: columnMapTargets(external as AnyRec, undeclared.column) }; +} + +/** + * [#27425] The lint's reading of plugin-sharing's `hasPhantomOwnerAnchor`: an + * external object whose `owner_id` is the platform's injected anchor (no + * declared field) and no `external.columnMap` entry targets it. No owner is + * provable for such an object, so record sharing admits no row by ownership on + * an owner-scoped model, whatever the binding's writability. + */ +export function hasUnprovableOwnerAnchor(obj: AnyRec): boolean { + const external = obj.external; + if (!external || typeof external !== 'object' || Array.isArray(external)) return false; + if (columnMapTargets(external as AnyRec, OWNER_COLUMN)) return false; + return resolveInjectedColumnProvenance(obj, OWNER_COLUMN) === 'injected-unprovisioned'; } export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharingFinding[] { @@ -188,16 +221,13 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin const undeclared = externalOwnershipGap(obj, readOnlyDatasources); if (!undeclared) return; - const { column } = undeclared; - const external = obj.external as AnyRec; + const { column, mapped } = undeclared; const name = typeof obj.name === 'string' ? obj.name : `(object ${i})`; const model = typeof obj.sharingModel === 'string' ? `'${obj.sharingModel}'` : "'private' (the default)"; const check = column === OWNER_COLUMN ? 'record sharing reads owner_id' : 'with no owner_id, the ownership floor reads created_by == the caller'; - const mapped = columnMapTargets(external, column); - const role = column === OWNER_COLUMN ? 'owner' : 'creator'; // [#27425] On `private`, record sharing also scopes READS by owner_id, and // an owner it cannot read admits no row: the edit's pre-image read finds // none, so the refusal is a 404 rather than the write check's 403. @@ -205,7 +235,7 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin column === OWNER_COLUMN && effectiveSharingModelOf(obj) === 'private' ? "(404: the member's read scope admits no row)" : '(403)'; - const declareWayOut = declareOwnershipColumnWayOut(column); + const declareWayOut = declareOwnershipColumnWayOut(column, mapped); out.push({ severity: 'warning', @@ -224,8 +254,7 @@ export function validateExternalOwnerSharing(stack: AnyRec): ExternalOwnerSharin `${check} before every edit and delete, so every member write is refused ${refusal} unless an app-authored ` + 'row-level policy admits the row', hint: mapped - ? `Declare ${column} in fields (a plugin driver must return the remote ${role} under that name itself), ` + - `or ${PUBLIC_READ_WRITE_WAY_OUT}` + ? `D${declareWayOut.slice(1)}, or ${PUBLIC_READ_WRITE_WAY_OUT}` : `Either ${PUBLIC_READ_WRITE_WAY_OUT}, or ${declareWayOut}`, }); }); diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 0d7853b3421..f3aa680bc2b 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -148,6 +148,7 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { // [#27472] The writable-external exception to the 'private' recommendation, and the rule it defers to. expect(text).toContain('external.writable: true'); expect(text).toContain('os explain security-external-owner-unmapped'); + expect(text).toContain("is told to declare 'public_read' instead"); expect(finding.message).not.toContain('incident'); }); diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index ce9dd0f8e85..b5bf9506e68 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -127,8 +127,10 @@ import { indexObjectGraph, nearestName, packagesOf, recordsOf, type ObjectGraph import { injectedColumnsFor } from './system-fields.js'; import { PUBLIC_READ_WRITE_WAY_OUT, + READ_ONLY_EXTERNAL_WAY_OUT, declareOwnershipColumnWayOut, externalOwnershipGap, + hasUnprovableOwnerAnchor, readOnlyExternalDatasources, } from './validate-external-owner-sharing.js'; @@ -810,12 +812,17 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } if (!isSystemObject(obj)) { if (owd == null) { - // [#27472] On a writable external object whose ownership column no - // remote row serves, 'private' is the shape that refuses every member - // edit (`security-external-owner-unmapped`), so the fix line names that - // rule's ways out instead of recommending it. Every other object — - // an external one that declares its owner column included — keeps the - // 'private' recommendation. + // [#27472] 'private' is never recommended where it refuses the member: + // - a writable external binding whose ownership column no remote row + // serves gets `security-external-owner-unmapped`'s two ways out; + // - any other external object whose owner anchor is unprovable (a + // read-only binding, or one on a datasource that lets no write + // through) gets 'public_read': on 'private' a member reads no row + // (#27425), whatever the binding's writability. The publish gate + // sees no datasources, so there an allowWrites: false datasource + // reads as writable; both doors still never say 'private'. + // Every other object, an external one that declares its owner + // included, keeps the 'private' recommendation. const ownershipGap = externalOwnershipGap(obj, readOnlyDatasources); findings.push({ severity: 'error', @@ -830,10 +837,13 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } `custom object declares no sharingModel (OWD); the runtime falls back to 'private', ` + `but the baseline must be an authored decision`, hint: ownershipGap - ? `${PUBLIC_READ_WRITE_WAY_OUT}, or ${declareOwnershipColumnWayOut(ownershipGap.column)} ` + + ? `${PUBLIC_READ_WRITE_WAY_OUT}, or ` + + `${declareOwnershipColumnWayOut(ownershipGap.column, ownershipGap.mapped)} ` + `and declare sharingModel: 'private' or 'public_read'; 'controlled_by_parent' for master-detail children` - : `declare sharingModel: 'private' (owner + shares; recommended), 'public_read', ` + - `'public_read_write', or 'controlled_by_parent' (master-detail children)`, + : hasUnprovableOwnerAnchor(obj) + ? READ_ONLY_EXTERNAL_WAY_OUT + : `declare sharingModel: 'private' (owner + shares; recommended), 'public_read', ` + + `'public_read_write', or 'controlled_by_parent' (master-detail children)`, }); } else if (typeof owd === 'string' && OWD_ALIAS_FIX[owd]) { // Reachable ONLY through the unparsed doors (`os lint` on a raw From 78e68ad43b69cdfc71665680aa652e25e3635ccc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:06:50 +0000 Subject: [PATCH 5/5] chore(lint): changeset and test title cover the read-only external case Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV --- .changeset/27472-owd-unset-external-hint.md | 9 +++++---- .../lint/src/validate-external-owner-sharing.test.ts | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.changeset/27472-owd-unset-external-hint.md b/.changeset/27472-owd-unset-external-hint.md index 531897cce38..f2aa888da53 100644 --- a/.changeset/27472-owd-unset-external-hint.md +++ b/.changeset/27472-owd-unset-external-hint.md @@ -9,8 +9,9 @@ Metadata-protocol: none `security-owd-unset` refuses a custom object with no `sharingModel`, and its fix line recommended `sharingModel: 'private'` for every such object. On a writable external object (`external.writable: true`) whose ownership column no remote row serves, `'private'` is the shape `security-external-owner-unmapped` warns refuses every member edit: the author followed one rule's advice into the other's warning. -- **Now.** For exactly the objects `security-external-owner-unmapped` judges (a writable binding, on a datasource this stack does not declare with `external.allowWrites: false`, with no declared `owner_id`, or `created_by` on an object without `owner_id`), the fix line reads: declare `sharingModel: 'public_read_write'` and decide who may edit in a `beforeUpdate` hook, or declare the ownership column in `fields` and have the driver return the remote owner under that name, and declare `'private'` or `'public_read'`; `'controlled_by_parent'` for master-detail children. Both clauses are that rule's own wording. -- **Unchanged.** The severity (`error`) and the verdict line. Every other custom object, and an external object that already declares its ownership column, keeps the `'private'` recommendation. -- `os explain security-owd-unset` explains the writable-external exception and points at `os explain security-external-owner-unmapped`. +- **Now, on a writable binding.** For exactly the objects `security-external-owner-unmapped` judges (`external.writable: true`, with no declared `owner_id`, or `created_by` on an object without `owner_id`), the fix line reads: declare `sharingModel: 'public_read_write'` and decide who may edit in a `beforeUpdate` hook, or declare the ownership column in `fields` and have the driver return the remote owner under that name, and declare `'private'` or `'public_read'`; `'controlled_by_parent'` for master-detail children. Both clauses are that rule's own wording, including its shorter one when an `external.columnMap` entry already targets the column. +- **Now, on a read-only binding.** An external object that no member writes (`external.writable` not `true`, or a datasource this stack declares with `external.allowWrites: false`) and whose `owner_id` no declared field or `external.columnMap` entry names is told to declare `sharingModel: 'public_read'`: on `'private'` a member reads none of its rows. +- **Unchanged.** The severity (`error`) and the verdict line. Every other custom object, and an external object whose owner is provable, keeps the `'private'` recommendation. +- `os explain security-owd-unset` explains both external cases and points at `os explain security-external-owner-unmapped`. -The same fix line reaches the runtime publish gate's 422 issue for an `object` write. +The same fix line reaches the runtime publish gate's 422 issue for an `object` write. That gate sees no `datasources`, so a writable binding on an `allowWrites: false` datasource gets the writable-binding line there; neither door recommends `'private'` for it. diff --git a/packages/lint/src/validate-external-owner-sharing.test.ts b/packages/lint/src/validate-external-owner-sharing.test.ts index 10bcce6fbd9..a36f2e8a788 100644 --- a/packages/lint/src/validate-external-owner-sharing.test.ts +++ b/packages/lint/src/validate-external-owner-sharing.test.ts @@ -229,7 +229,7 @@ describe('security-external-owner-unmapped — the fix line', () => { // to recommend 'private' everywhere: on a writable external object that is the shape this rule warns // refuses every member edit. Its fix line for the objects this rule judges is built from this // rule's own two ways out; every other object keeps the 'private' recommendation. -describe("security-owd-unset's fix line on a writable external object", () => { +describe("security-owd-unset's fix line on an external object", () => { const owdUnsetHint = (stack: Record, where: string) => { const findings = validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_OWD_UNSET && f.where === where); expect(findings, where).toHaveLength(1);