diff --git a/.changeset/fls-keys-release-sync-d11.md b/.changeset/fls-keys-release-sync-d11.md new file mode 100644 index 0000000000..12f3b820c3 --- /dev/null +++ b/.changeset/fls-keys-release-sync-d11.md @@ -0,0 +1,34 @@ +--- +"@objectstack/lint": minor +"@objectstack/example-showcase": patch +"@objectstack/spec": patch +--- + +Permission-zoo audit follow-ups: + +**FLS keys must be object-qualified (`security-fls-unqualified-key`, error).** +The runtime evaluator matches field-permission keys by `.` +prefix — a bare `budget` key matches NOTHING and the declared masking +silently never enforces. The showcase itself shipped exactly that bug: its +contributor FLS block (bare `budget`/`spent`/`budget_remaining`) was a +runtime no-op, and the "FLS proof" in earlier verification was actually a +validation-rule rejection. Fixed: keys qualified +(`showcase_project.budget` …), a new D7 lint rule rejects bare keys at +compile time with a fix-it, and the permission-zoo dogfood now proves the +served pipeline denies a contributor's budget write while allowing ordinary +field edits. + +**Release pipeline: PROTOCOL_VERSION auto-sync.** `changeset version` now +runs `scripts/sync-protocol-version.mjs`, regenerating the handshake +constant from the spec package major. Release PRs opened by +changesets/action with the default GITHUB_TOKEN never trigger CI (GitHub's +anti-recursion rule), so the lockstep guard could only fire AFTER a release +merged — the drift class that broke main at 14.0.0 (#2769) is now fixed at +version time, the one spot that cannot be skipped. + +**D11 `externalSharingModel` honestly marked.** The dial has no runtime +consumer yet (authoring lint + Studio badges only); its liveness entry +moves from a bespoke `authorable` status to the documented `planned` + +`authorWarn`, and the sharing docs / design doc / showcase comments now say +explicitly that evaluation of external principals lands with the +principal-taxonomy phase (#2696). diff --git a/content/docs/permissions/sharing-rules.mdx b/content/docs/permissions/sharing-rules.mdx index 6191c95480..120a74f1fe 100644 --- a/content/docs/permissions/sharing-rules.mdx +++ b/content/docs/permissions/sharing-rules.mdx @@ -56,6 +56,13 @@ externalSharingModel: 'private', // external principals: own + shares onl - "External" is a property of the **principal** (`audience: 'external'`), never of the object. The BU depth axis does not apply to externals; their visibility = own records + explicit shares + the external OWD. +- **Status: declared, not yet evaluated at runtime.** Today the dial is + authoring-validated (the linter above) and surfaced in Studio (the `Ext` + badge per object row); the evaluator branch that substitutes it for + external principals lands with the principal-taxonomy semantics phase + (tracked on #2696, liveness: `planned`). Until then no request is + evaluated as external — authoring the dial prepares your model without + changing behavior. ## Criteria-Based Sharing Rules diff --git a/docs/design/permission-model.md b/docs/design/permission-model.md index e7dda352ec..181f85814a 100644 --- a/docs/design/permission-model.md +++ b/docs/design/permission-model.md @@ -110,6 +110,10 @@ declare `externalSharingModel` (same four values, ADR-0090 D11). It defaults to **never be wider than the internal one** — e.g. accounts can be `public_read` for employees while staying `private` for dealer-portal users, who then see only records they own or were explicitly shared. If your object has no external audience, ignore this field entirely. +*Status:* the dial is authoring-validated (D7 lint `security-external-wider-than-internal`) and +surfaced in Studio; the runtime evaluator branch that applies it to `audience: 'external'` +principals lands with the principal-taxonomy semantics phase (#2696, liveness `planned` + +author-warning). No request evaluates as external today. ## 4. Worked example — a CRM package diff --git a/examples/app-showcase/src/data/objects/account.object.ts b/examples/app-showcase/src/data/objects/account.object.ts index 8145b9e039..59a294a4e2 100644 --- a/examples/app-showcase/src/data/objects/account.object.ts +++ b/examples/app-showcase/src/data/objects/account.object.ts @@ -21,6 +21,8 @@ export const Account = ObjectSchema.create({ // [ADR-0090 D11] External principals may READ accounts (portal users see // the customer directory) but never write them — a non-default external // dial that is still strictly ≤ the internal `public_read_write`. + // Authoring-validated + Studio-surfaced today; runtime evaluation lands + // with the principal-taxonomy phase (#2696, liveness `planned`). externalSharingModel: 'public_read', label: 'Account', pluralLabel: 'Accounts', diff --git a/examples/app-showcase/src/data/objects/announcement.object.ts b/examples/app-showcase/src/data/objects/announcement.object.ts index 0ed9376792..0ec0fa42ba 100644 --- a/examples/app-showcase/src/data/objects/announcement.object.ts +++ b/examples/app-showcase/src/data/objects/announcement.object.ts @@ -28,7 +28,9 @@ export const Announcement = ObjectSchema.create({ // announcements are public_read for employees but PRIVATE to external // users — an external portal user sees only announcements they own or were // explicitly shared. Must never be wider than the internal model - // (validated: external ≤ internal). + // (validated: external ≤ internal). NOTE the dial is authoring-validated + + // Studio-surfaced today; runtime evaluation of external principals lands + // with the principal-taxonomy phase (#2696, liveness `planned`). externalSharingModel: 'private', fields: { diff --git a/examples/app-showcase/src/security/permission-sets.ts b/examples/app-showcase/src/security/permission-sets.ts index 93069c7fce..2aa50a78b1 100644 --- a/examples/app-showcase/src/security/permission-sets.ts +++ b/examples/app-showcase/src/security/permission-sets.ts @@ -44,10 +44,14 @@ export const ContributorPermissionSet = definePermissionSet({ showcase_invoice_line: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false }, }, // Field-level security — contributors can read but not edit budget figures. + // Keys MUST be `.` qualified: the runtime evaluator matches + // by object prefix, so a bare `budget` key silently enforces NOTHING (the + // showcase shipped exactly that bug — caught by the permission-zoo audit, + // now rejected at compile time by `security-fls-unqualified-key`). fields: { - budget: { readable: true, editable: false }, - spent: { readable: true, editable: false }, - budget_remaining: { readable: true, editable: false }, + 'showcase_project.budget': { readable: true, editable: false }, + 'showcase_project.spent': { readable: true, editable: false }, + 'showcase_project.budget_remaining': { readable: true, editable: false }, }, // Row-level security — contributors only see tasks assigned to them. rowLevelSecurity: [ diff --git a/package.json b/package.json index dac1db4f06..1364e2d4b9 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "test:e2e": "turbo run test:e2e", "clean": "turbo run clean && rm -rf dist", "setup": "pnpm install && pnpm --filter @objectstack/spec build", - "version": "changeset version", + "version": "changeset version && node scripts/sync-protocol-version.mjs", "release": "pnpm run build && bash scripts/build-console.sh && bash scripts/release-publish.sh", "docs:dev": "pnpm --filter @objectstack/docs dev", "docs:build": "pnpm --filter @objectstack/docs build", diff --git a/packages/dogfood/test/showcase-permission-zoo.dogfood.test.ts b/packages/dogfood/test/showcase-permission-zoo.dogfood.test.ts index 451c608639..76ba262f67 100644 --- a/packages/dogfood/test/showcase-permission-zoo.dogfood.test.ts +++ b/packages/dogfood/test/showcase-permission-zoo.dogfood.test.ts @@ -211,4 +211,40 @@ describe('showcase: ADR-0090 permission-model zoo', () => { }); expect(r.status, 'read-only + create-intake set passes the guest tier').toBeLessThan(300); }); + + // ── FLS enforcement — object-qualified keys (permission-zoo audit) ─────── + // The contributor set masks the project budget figures read-only. The keys + // were originally BARE (`budget`) — which the evaluator silently ignores + // (it matches `.` prefixes), so the declared masking never + // enforced. Qualified keys + the `security-fls-unqualified-key` lint close + // that; this proves the served pipeline actually denies the write now. + it('FLS: a contributor can edit a project but NOT its budget fields', async () => { + const plain = await ql.findOne('sys_user', { where: { email: 'zoo-plain@verify.test' }, context: SYS }); + const contribSet = await ql.findOne('sys_permission_set', { where: { name: 'showcase_contributor' }, context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: plain.id, permission_set_id: contribSet.id }, { context: SYS }) + .catch(() => { /* already granted by an earlier step — idempotent */ }); + + const projects = await ql.find('showcase_project', { limit: 1, context: SYS }); + const projectId = projects?.[0]?.id; + expect(projectId, 'seeded project available').toBeTruthy(); + const budgetBefore = projects[0].budget; + + // Editable field → allowed (allowEdit on showcase_project). + const nameEdit = await stack.apiAs(plainTok, 'PATCH', `/data/showcase_project/${projectId}`, { + name: 'Renamed by contributor (FLS probe)', + }); + expect(nameEdit.status, 'ordinary field edit passes').toBeLessThan(300); + + // Masked field → denied. A LARGE budget keeps the spent_within_budget + // validation rule satisfied, so a rejection here can only be the FLS + // write-deny — before the key qualification this returned 2xx (the bare + // key was silently ignored). + const budgetEdit = await stack.apiAs(plainTok, 'PATCH', `/data/showcase_project/${projectId}`, { + budget: 999999999, + }); + expect(budgetEdit.status, 'editable:false field write is denied').not.toBeLessThan(300); + + const after = await ql.findOne('showcase_project', { where: { id: projectId }, context: SYS }); + expect(after.budget, 'budget unchanged after the denied write').toBe(budgetBefore); + }); }); diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index d3c3cf4a7d..b02f1dde67 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect } from 'vitest'; import { + SECURITY_FLS_UNQUALIFIED_KEY, validateSecurityPosture, SECURITY_OWD_UNSET, SECURITY_OWD_ALIAS, @@ -122,6 +123,37 @@ describe('validateSecurityPosture (ADR-0090 D7)', () => { ).toEqual([]); }); + // ── Rule: security-fls-unqualified-key (permission-zoo audit) ─────── + it('errors on a bare FLS key — the runtime evaluator silently ignores it', () => { + const findings = validateSecurityPosture({ + permissions: [ + { + name: 'contrib', + objects: { crm_opportunity: { allowRead: true, allowEdit: true } }, + fields: { cost_internal: { readable: true, editable: false } }, + }, + ], + }).filter((f) => f.rule === SECURITY_FLS_UNQUALIFIED_KEY); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toMatch(/silently IGNORED/); + expect(findings[0].hint).toMatch(/crm_opportunity\.cost_internal/); + }); + + it('accepts object-qualified FLS keys', () => { + expect( + rulesOf({ + permissions: [ + { + name: 'contrib', + objects: { crm_opportunity: { allowRead: true } }, + fields: { 'crm_opportunity.cost_internal': { readable: false, editable: false } }, + }, + ], + }), + ).toEqual([]); + }); + // ── Rule: security-anchor-high-privilege (ADR-0090 D5/D9) ─────────── it('errors when an isDefault (everyone-suggested) set carries high-privilege bits', () => { const findings = validateSecurityPosture({ diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index a895e89ff7..420cb8cefe 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -44,6 +44,7 @@ export const SECURITY_ROLE_WORD = 'security-role-word'; export const SECURITY_BOOK_AUDIENCE_UNKNOWN_SET = 'security-book-audience-unknown-set'; export const SECURITY_PRIVATE_NO_READSCOPE = 'security-private-no-readscope'; export const SECURITY_MASTER_DETAIL_UNGRANTED = 'security-master-detail-ungranted'; +export const SECURITY_FLS_UNQUALIFIED_KEY = 'security-fls-unqualified-key'; export const SECURITY_GRANT_EXPIRED_AT_AUTHORING = 'security-grant-expired-at-authoring'; export const SECURITY_DELEGATION_MISSING_REASON = 'security-delegation-missing-reason'; @@ -252,6 +253,27 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } const psPath = `permissions[${i}]`; const objectsMap = (ps.objects && typeof ps.objects === 'object' ? ps.objects : {}) as AnyRec; + // [#19 / permission zoo audit] FLS keys MUST be `.` + // qualified. The runtime evaluator matches keys by object prefix + // (`getFieldPermissions`: `key.startsWith(objectName + '.')`), so a bare + // `budget` key matches NOTHING — the declared masking silently never + // enforces (the worst declared-≠-enforced class, ADR-0049). The showcase + // itself shipped this bug for months. + const flsMap = (ps.fields && typeof ps.fields === 'object' ? ps.fields : {}) as AnyRec; + for (const flsKey of Object.keys(flsMap)) { + if (flsKey.includes('.')) continue; + findings.push({ + severity: 'error', + rule: SECURITY_FLS_UNQUALIFIED_KEY, + where: `permission set "${psName}"`, + path: `${psPath}.fields["${flsKey}"]`, + message: + `field-permission key '${flsKey}' is not object-qualified — the runtime matches FLS keys ` + + `by '.' prefix, so a bare key is silently IGNORED and the declared masking never enforces.`, + hint: `Qualify the key with its object, e.g. 'crm_opportunity.${flsKey}': { readable: true, editable: false }.`, + }); + } + const wildcard = objectsMap['*'] as AnyRec | undefined; if (wildcard && (wildcard.viewAllRecords === true || wildcard.modifyAllRecords === true)) { findings.push({ diff --git a/packages/spec/liveness/object.json b/packages/spec/liveness/object.json index 4f37ff7f1f..85451cb58c 100644 --- a/packages/spec/liveness/object.json +++ b/packages/spec/liveness/object.json @@ -248,8 +248,9 @@ "note": "objectql `$search` executor (ADR-0061: expandSearchToFilter in engine.find) + objectui list/lookup/command-palette; canonical searchable-field source." }, "externalSharingModel": { - "status": "authorable", - "note": "[ADR-0090 D11] P1 lands the SPEC SHAPE only (validated external<=internal at authoring). Runtime consumption (audience-aware evaluator branch) is scheduled with the principal-taxonomy semantics phase; tracked on the ADR-0090 tracking issue (#2696)." + "status": "planned", + "authorWarn": true, + "note": "[ADR-0090 D11] P1 lands the SPEC SHAPE only (validated external<=internal at authoring; Studio surfaces the Ext badge). Runtime consumption (audience-aware evaluator branch substituting the external dial for external principals) is scheduled with the principal-taxonomy semantics phase; tracked on #2696. `planned` + authorWarn per enforce-or-mark: authors get told the dial does not evaluate yet. Was a bespoke 'authorable' status outside the documented vocabulary." } } } diff --git a/scripts/sync-protocol-version.mjs b/scripts/sync-protocol-version.mjs new file mode 100644 index 0000000000..469196e48f --- /dev/null +++ b/scripts/sync-protocol-version.mjs @@ -0,0 +1,43 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Re-sync the PROTOCOL_VERSION handshake constant with @objectstack/spec's +// package major. Runs as part of the root `version` script (changesets/action +// calls `pnpm run version` when preparing the release PR), so a spec major +// bump can never ship without the constant following — the drift class that +// broke main after the 14.0.0 release (#2769): the lockstep guard +// (protocol-version.test.ts) exists, but release PRs opened by +// changesets/action with the default GITHUB_TOKEN do not trigger CI (GitHub's +// anti-recursion rule), so the guard only fired AFTER the merge. Fixing the +// value at version time is the only spot that cannot be skipped. + +import { readFileSync, writeFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; + +const root = dirname(dirname(fileURLToPath(import.meta.url))); +const pkgPath = join(root, 'packages/spec/package.json'); +const constPath = join(root, 'packages/spec/src/kernel/protocol-version.ts'); + +const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')); +const major = Number.parseInt(String(pkg.version).split('.')[0], 10); +if (!Number.isInteger(major) || major < 1) { + console.error(`✗ sync-protocol-version: cannot parse major from spec version '${pkg.version}'`); + process.exit(1); +} + +const src = readFileSync(constPath, 'utf8'); +const re = /export const PROTOCOL_VERSION = '(\d+)\.0\.0';/; +const m = src.match(re); +if (!m) { + console.error(`✗ sync-protocol-version: PROTOCOL_VERSION declaration not found in ${constPath}`); + process.exit(1); +} + +const current = Number.parseInt(m[1], 10); +if (current === major) { + console.log(`✓ PROTOCOL_VERSION already ${major}.0.0 — in lockstep with @objectstack/spec@${pkg.version}`); + process.exit(0); +} + +writeFileSync(constPath, src.replace(re, `export const PROTOCOL_VERSION = '${major}.0.0';`)); +console.log(`✓ PROTOCOL_VERSION ${current}.0.0 → ${major}.0.0 (lockstep with @objectstack/spec@${pkg.version})`);