Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .changeset/sharing-rule-inert-anchor-gate.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
"@objectstack/lint": minor
---

feat(lint): a sharing rule anchored where sharing has nothing to widen is now an authoring-time error (#9698)

`validateSharingRuleEnforceability` gains its second arm. It already judged a
sharing rule's `condition` against the compiler that lowers it; it now judges
the rule's `object` against the verdict that decides whether the grant can
exist at all.

Two new `error` ids, both decidable from authored metadata before anything
boots, and both mirroring `SharingService.inertGrantReason` (ADR-0111 D7)
rather than modelling it:

- **`sharing-rule-object-not-shareable`** — the anchor object's effective
sharing model is `public` (an explicit `sharingModel: 'public_read_write'`,
or no `sharingModel` on a system object, which ADR-0090 D1 resolves to
public). Sharing only ever WIDENS an OWD baseline, so on the widest baseline
there is nothing to widen.
- **`sharing-rule-object-controlled-by-parent`** — the anchor is a
master-detail detail, whose visibility is derived from its master
(ADR-0055). It gets its own id and its own fix-it ("share the master
record instead"), because `effectiveSharingModel` collapses it onto the same
`public` verdict while the correct repair is completely different.

Both were previously accepted by `SharingRuleSchema`, accepted by `defineRule`,
seeded into `sys_sharing_rule`, and only then refused — once per boot, as a
WARN line inside the boot diagnostics block. That WARN is not a sufficient
diagnostic, and the reason is measured rather than argued: a rule whose criteria
match no seeded row never reaches `grant`, so it never throws and warns nothing
while being exactly as dead. The WARN is a function of the DATA; the defect is a
property of the DECLARATION.

**Blast radius, measured through `objectstack build` before deciding the
severity:** 5 sharing rules are declared in this repo. 3 fire, all of them in
`examples/app-crm` — `share_high_value_opps_with_managers`,
`share_active_leads_with_manager` and `share_won_deal_activities`, anchored on
`crm_opportunity`, `crm_lead` and `crm_activity`, every one of them
`sharingModel: 'public_read_write'`. They have been failing their boot backfill
on every boot of that app since they were written, and they are removed here
under ADR-0049 enforce-or-remove — the same call #9237 made for the two
equivalent rules in `app-showcase`. The other 2 (app-showcase's, both on
`private` objects) stay silent, which is the direction that had to be proven
rather than hoped for.

The CRM's smoke test used to assert that these rules existed and were of the
enforced `criteria` type. Both assertions passed while all three rules enforced
nothing, so the assertion is replaced by the property their greenness hid: no
declared rule may be anchored where sharing has nothing to widen.

Deliberately NOT judged, because they are not decidable from authored metadata:
the `owner_id` arm (`owner_id` is injected by the schema registry, so asserting
it would fail every object that correctly does not declare it by hand), the
`bypassObjects` arm (plugin configuration, not stack metadata), and the
federated phantom-anchor arm (a provenance test over that same injected column).
24 changes: 15 additions & 9 deletions content/docs/permissions/index.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,22 +25,28 @@ is reserved (D3) — if you knew the v1 model, start at
[Positions](/docs/permissions/positions).

Access rules are metadata like everything else — this is a real sharing rule from
the CRM example app:
the showcase example app:

```typescript
export const HighValueOpportunitySharingRule = defineSharingRule({
export const KeyAccountQualifiedContactRule = defineSharingRule({
type: 'criteria',
name: 'share_high_value_opps_with_managers',
label: 'High-Value Deals → Sales Managers',
description: 'Automatically share opportunities over $100,000 with all Sales Managers.',
object: 'crm_opportunity',
condition: 'record.amount > 100000',
accessLevel: 'edit',
sharedWith: { type: 'position', value: 'sales_manager' },
name: 'share_key_account_qualified_contacts_with_managers',
label: 'Key-Account Qualified Contacts → Managers',
description: 'Share qualified contacts at the key account with managers.',
object: 'showcase_contact',
condition: "record.stage == 'qualified' && record.company == 'Northwind'",
accessLevel: 'read',
sharedWith: { type: 'position', value: 'manager' },
active: true,
});
```

Note the object it is anchored on. `showcase_contact` is OWD `private`, which is
what makes this grant one a read gate actually consults — sharing only ever
WIDENS a baseline, so a rule on an object that is already `public_read_write`
grants nothing and is refused at boot. That is an authoring-time build error
(`sharing-rule-object-not-shareable`), not a runtime surprise.

Because AI agents act through the same permission-aware surface, these rules bound
agent access exactly as they bound users ([Actions as Tools](/docs/ai/actions-as-tools)).

Expand Down
23 changes: 15 additions & 8 deletions examples/app-crm/objectstack.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,9 +17,6 @@ import {
FinanceApproverPosition,
SalesUserPermissionSet,
GuestPortalProfile,
HighValueOpportunitySharingRule,
RepLeadSharingRule,
WonDealActivitySharingRule,
} from './src/security/index.js';
import { registerCrmPositionBindings } from './src/security/bind-position-sets.js';
import { CrmSeedData } from './src/data/index.js';
Expand DownExpand Up@@ -102,11 +99,21 @@ export default defineStack({
// Security
positions: [SalesRepPosition, SalesManagerPosition, FinanceApproverPosition],
permissions: [SalesUserPermissionSet, GuestPortalProfile],
sharingRules: [
HighValueOpportunitySharingRule,
RepLeadSharingRule,
WonDealActivitySharingRule,
],
// No `sharingRules`. The three this app used to declare
// (`share_high_value_opps_with_managers`, `share_active_leads_with_manager`,
// `share_won_deal_activities`) were anchored on `crm_opportunity`,
// `crm_lead` and `crm_activity` — all three `sharingModel:
// 'public_read_write'`. Sharing only ever WIDENS an OWD baseline, so on the
// widest baseline there is nothing to widen: `assertNotInertGrant` refused
// every grant with SHARING_NOT_ENABLED and the boot backfill failed for each
// rule, on every boot, since they were written. They granted nothing and
// were removed under ADR-0049 enforce-or-remove (#9698), the same call
// #9237 made for app-showcase's two.
//
// ⛔ Do not re-add one on a public object — `sharing-rule-object-not-shareable`
// now fails the build, and its message states the two honest fixes. Giving
// this app a LIVE sharing demonstration means giving it a `private` object
// first; that is an access-matrix change (ADR-0090 D6 review), not a rider.

// Seed data
data: CrmSeedData,
Expand Down
6 changes: 0 additions & 6 deletions examples/app-crm/src/security/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,9 +8,3 @@ export {
GuestPortalProfile,
} from './sales-positions.js';

export {
HighValueOpportunitySharingRule,
RepLeadSharingRule,
WonDealActivitySharingRule,
} from './sharing-rules.js';

65 changes: 0 additions & 65 deletions examples/app-crm/src/security/sharing-rules.ts

This file was deleted.

33 changes: 28 additions & 5 deletions examples/app-crm/test/smoke.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -93,11 +93,34 @@ describe('app-crm minimal metadata bundle', () => {
expect(stack.i18n!.supportedLocales).toContain('zh-CN');
});

it('has criteria sharing rules (the enforced form — owner-type was retired)', () => {
const rules = stack.sharingRules ?? [];
expect(rules.length).toBeGreaterThanOrEqual(2);
// `type: 'owner'` no longer parses (never enforced; ADR-0078): every
// declared rule is the enforced criteria form.
// #9698 — this used to assert `rules.length >= 2` and `every(type ===
// 'criteria')`. Both passed, and neither was the property that mattered:
// all three declared rules were anchored on `public_read_write` objects, so
// `assertNotInertGrant` refused every grant and the boot backfill failed for
// each one, on every boot. A test can assert the enforced FORM and stay
// green while the rule enforces nothing. The three were removed (ADR-0049
// enforce-or-remove); what replaces the assertion is the property their
// greenness hid, so re-adding one on a public object goes red HERE as well
// as at `objectstack build`.
it('no declared sharing rule is anchored where sharing has nothing to widen (#9698)', () => {
const rules = (stack.sharingRules ?? []) as Array<{ name?: string; object?: string; type?: string }>;
const owdOf = new Map(
((stack.objects ?? []) as Array<{ name?: string; sharingModel?: string }>)
.map((o) => [String(o.name), o.sharingModel]),
);
// `effectiveSharingModel` maps BOTH to 'public'; `controlled_by_parent`
// earns its own runtime refusal ("share the master record instead").
const INERT_OWD = new Set(['public_read_write', 'controlled_by_parent']);
const offenders = rules
.filter((r) => INERT_OWD.has(String(owdOf.get(String(r.object)))))
.map((r) => `${r.name} → ${r.object} (sharingModel '${owdOf.get(String(r.object))}')`);
expect(
offenders,
`sharing rule(s) whose grant no gate would consult — the boot backfill refuses these with `
+ `SHARING_NOT_ENABLED: ${offenders.join(', ')}`,
).toEqual([]);
// `type: 'owner'` no longer parses (never enforced; ADR-0078): whatever is
// declared is the enforced criteria form.
expect(rules.every((r) => r.type === 'criteria')).toBe(true);
});

Expand Down
45 changes: 30 additions & 15 deletions packages/lint/src/authoring-rules.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1306,16 +1306,26 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT,
run: (stack) => validateOrgAxisRedLines(stack),
},
// #4698 — the "declared but never read" gate, for the one surface where the
// predicate is EXACT rather than inferred. A sharing rule's `condition` has a
// single runtime consumer (`bootstrapDeclaredSharingRules`) whose only use of
// the key is `compileCelToFilter(condition, { variables: {} })`; a condition
// that does not lower means the rule is SKIPPED at boot, so the grant is
// declared and does not exist. The lint calls that same compiler, from the
// same package, with the same options — the verdict cannot drift from the
// consumer's. Gating for the ADR-0078 reason `SharingRuleSchema`'s own
// docblock states: the whole authorable surface is enforced, and this was the
// one field where that sentence was not yet true.
// #4698 / #9698 — the "declared but never read" gate, for the two fields of a
// sharing rule where the predicate is EXACT rather than inferred.
//
// - `condition` has a single runtime consumer
// (`bootstrapDeclaredSharingRules`) whose only use of the key is
// `compileCelToFilter(condition, { variables: {} })`; a condition that
// does not lower means the rule is SKIPPED at boot.
// - `object` decides whether the grant is refused outright: reconcile hands
// each row to `SharingService.grant`, whose ADR-0111 D7 pre-flight THROWS
// `SHARING_NOT_ENABLED` when the anchor's effective sharing model is
// `public` or it is a `controlled_by_parent` detail. Both are decidable
// from authored metadata; the other arms of that verdict (`owner_id`, the
// bypass set, federated anchors) are not, and are excluded by name.
//
// Either way the grant is declared and does not exist. The lint calls the
// same compiler and mirrors the same verdict function, from the same inputs
// — the verdict cannot drift from the consumers'. Gating for the ADR-0078
// reason `SharingRuleSchema`'s own docblock states: the whole authorable
// surface is enforced, and these were the fields where that sentence was not
// yet true.
{
name: 'validateSharingRuleEnforceability',
tier: 'gating',
Expand All@@ -1324,11 +1334,16 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
source: 'packages/lint/src/validate-sharing-rule-enforceability.ts',
surfaces: CLI_ONLY,
surfaceReason:
'P2 (#4463): a sharing rule is not a `flow`, and P1 gates `flow` alone. The rule itself is '
+ 'snapshot-safe — it reads ONLY `stack.sharingRules[].condition` and needs no other collection — '
+ 'so widening it here is a `runtimeTypes: [\'sharing_rule\']` edit once the gate accepts that type, '
+ 'not new wiring. Recorded as pending rather than done, because a rule that has never run at a '
+ 'door should not claim it.',
'P2 (#4463): a sharing rule is not a `flow`, and P1 gates `flow` alone. This entry used to add '
+ 'that the rule reads ONLY `stack.sharingRules[].condition` and needs no other collection, so '
+ 'crossing was a lone `runtimeTypes` edit. #9698 FALSIFIED that: the anchor arm resolves '
+ '`sharingRules[].object` against `stack.objects` to read the anchor\'s OWD, so the rule is now '
+ 'cross-collection. `objects` IS carried by the per-write snapshot (`CONTEXT_STACK_KEYS`, #8309), '
+ 'so the remaining gap is unchanged in SHAPE — the gate must accept a `sharing_rule` type and the '
+ 'snapshot must carry `sharingRules`, which it does not — but it is now TWO collections, not one. '
+ 'Crossing with `sharingRules` uncarried would enforce this id for zero of its inputs while the '
+ 'entry claimed the door (#7220). Recorded as pending rather than done, because a rule that has '
+ 'never run at a door should not claim it.',
run: (stack) => validateSharingRuleEnforceability(stack),
},
// #4983 — the sibling surface of the rule above, and ADR-0056 D4's gate,
Expand Down
12 changes: 8 additions & 4 deletions packages/lint/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -261,14 +261,18 @@ export {
} from './validate-org-axis-red-lines.js';
export type { OrgAxisFinding, OrgAxisSeverity } from './validate-org-axis-red-lines.js';

// #4698 — "a key that nothing reads should not validate clean", for the one
// surface where "is it read?" is decidable: a sharing rule's `condition` is
// read ONLY through `compileCelToFilter`, so the lint calls that same compiler
// rather than modelling the consumer.
// #4698 / #9698 — "a key that nothing reads should not validate clean", for the
// two fields of a sharing rule where "is it read?" is decidable. The
// `condition` is read ONLY through `compileCelToFilter`, and the `object`
// decides whether `assertNotInertGrant` would refuse the grant outright — so
// the lint calls the same compiler and mirrors the same verdict function,
// rather than modelling either consumer.
export {
validateSharingRuleEnforceability,
SHARING_RULE_UNLOWERABLE_CONDITION,
SHARING_RULE_RUNTIME_VARIABLE_CONDITION,
SHARING_RULE_OBJECT_NOT_SHAREABLE,
SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT,
} from './validate-sharing-rule-enforceability.js';
export type {
SharingRuleEnforceabilityFinding,
Expand Down
Loading
Loading