From 5dd788b35e0ba0983bfe384b4f6722f2fea942df Mon Sep 17 00:00:00 2001 From: Warren Date: Tue, 1 Sep 2026 05:56:22 +0000 Subject: [PATCH] Declare hierarchy-security and restore the manager depth grants `objectstack.config.ts`, AGENTS.md rule 7 and two docs all said that declaring `requires: ['hierarchy-security']` would fail an open-edition boot. Measured false: it warns, naming the provider package, and every gate stays green. The claim is what forced the three manager depth grants down to `own`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- AGENTS.md | 50 ++++++++-- docs/deployment/security.md | 87 ++++++++++------- docs/product/data-model.md | 29 ++++-- objectstack.config.ts | 44 ++++++--- src/security/index.ts | 1 - src/security/permission-sets.ts | 117 +++++++++-------------- test/security.test.ts | 161 ++++++++++++++++++++++++++------ 7 files changed, 321 insertions(+), 168 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 762c455..2925349 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,14 +140,48 @@ rejects it. Use `position` (distribution), `permission_set` (capability), `status` and `due_date` directly — they are stored and indexed. 6. **`sharingModel` is mandatory and fail-closed.** Unset means private, and the publish linter errors on it (ADR-0090 D1/D7). State it deliberately. -7. **Author hierarchy scopes normally; the enterprise edition resolves them.** - `readScope: 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'` (ADR-0057) - are resolved by `@objectstack/security-enterprise`, which enterprise - deployments already carry. Do **not** build an application-level fallback, - and do **not** add `hierarchy-security` to `requires` — that would fail an - open-edition boot. In this open-edition checkout those scopes resolve to - owner-only, so a manager view will show you only your own rows: that is - expected here, and not a bug to chase. +7. **A hierarchy scope REQUIRES `requires: ['hierarchy-security']`. Omitting it + is an author-time hard error, not a silent fallback.** + `readScope`/`writeScope` `'own_and_reports' | 'unit' | 'unit_and_below'` + (ADR-0057) are the hierarchy scopes, resolved by + `@objectstack/security-enterprise`. `objectstack.config.ts` already declares + the capability — you should not need to touch it — and you author the scopes + normally. Build no application-level fallback. + + **Grant one without the declaration and `defineStack` refuses to load**, in + `validateHierarchyScopeCapability`. It is not the config file being fussy; + it is the platform closing the exact hole this rule used to tell you to live + with. The platform's own words: + + > A stack that uses one MUST declare `requires: ['hierarchy-security']`; + > otherwise the open runtime would silently fail closed to owner-only (the + > metadata would lie, ADR-0049). **This makes that an authoring-time error + > instead.** + + Because the check runs inside `defineStack()`, an undeclared scope takes + `validate`, `build` **and** every test that imports the config — so the + symptom is the whole suite going red at once, not one assertion. + + **Declaring it does NOT fail an open-edition boot.** Measured on this + checkout with `@objectstack/security-enterprise` **not** installed: + `validate`, `typecheck`, `test` and `build` all exit 0, the kernel logs + `Bootstrap complete`, and `validate` prints exactly one warning naming the + package that provides the capability. **That warning is the expected state + of this repo — do not silence it.** The only two ways to make it go away are + installing the enterprise package (deliberately not done here) and deleting + the declaration (which puts the hard error back). + + In this open-edition checkout the scopes then resolve to **owner-only**, so + a manager view shows you only your own rows. That is the edition, not a bug + to chase; verifying manager visibility for real needs an enterprise runtime + and a populated business-unit tree. + + ⛔ **`org` is not a hierarchy scope** and passes the check with no + declaration at all. That is the trapdoor: it is the nearest thing to hand + when a scope will not load and you want "some visibility for managers", and + it discloses every row in the tenant. Depth for managers is + `unit_and_below`. Narrower than intended is visible and fixable; + `org` is neither. 8. **English is the source language.** Every authored label gets an `en` entry; `zh-CN` is hand-translated. Do not hard-code display text in a hook or flow. 9. **Metadata first — a handler is the last resort, not the first.** This is an diff --git a/docs/deployment/security.md b/docs/deployment/security.md index cde8fef..5231c5f 100644 --- a/docs/deployment/security.md +++ b/docs/deployment/security.md @@ -1,9 +1,9 @@ # Security — what ships, and what a real rollout still has to do Duly declares three positions, three permission sets and no sharing rules. This -page is the other half: the bindings a package is not allowed to make, the -enterprise dependency a real deployment needs, and the two grants that are -currently narrower than the product intends. +page is the other half: the bindings a package is not allowed to make, and the +enterprise dependency a real deployment needs to resolve the manager depth this +package authors. Nothing here is advice about writing security code. There is no security code: `definePosition`, `definePermissionSet` and `defineSharingRule` are the whole @@ -18,8 +18,8 @@ apply over MCP, and never appears in an audit. | | `duly_member` | `duly_manager` | `duly_admin` | |:---|:---|:---|:---| | Who | everyone who owns duties | anyone with reports or a unit | catalog owners, rollout admins | -| `duly_task` | create / read / edit · read **own** · write **own** | inherited | inherited | -| `duly_duty` | create / read · read **own** · write **own** | inherited | **+ edit** · read **org** · write **own** | +| `duly_task` | create / read / edit · read **own** · write **own** | read **unit_and_below** · write **own** | inherited | +| `duly_duty` | create / read · read **own** · write **own** | read **unit_and_below** · write **own** | **+ edit** · read **org** · write **own** | | `duly_log_entry` | full control · read **own** · write **own** | inherited | inherited | | `duly_catalog_item` | read | inherited | **+ create / edit / delete** · write **org** | | `duly_assignment` | read · read **own** | **+ create / edit** · write **own** | inherited | @@ -85,10 +85,14 @@ Assignees still see their own fanned-out `duly_task`, which is the row they work ## An enterprise runtime is a product dependency -The manager model is built on the ADR-0057 depth scopes -(`own_and_reports`, `unit`, `unit_and_below`, `org`). Those are resolved by -**`@objectstack/security-enterprise`**. Without it the platform has no manager -chain and no business-unit tree resolver, so depth collapses to owner-only. +The manager model is built on the ADR-0057 hierarchy scopes +(`own_and_reports`, `unit`, `unit_and_below`) and this package authors them +directly: `duly_manager` reads `unit_and_below` on `duly_task` and `duly_duty`, +and `duly_admin` inherits the task depth. + +Those scopes are **resolved** by **`@objectstack/security-enterprise`**. Without +it the platform has no manager chain and no business-unit tree resolver, so +depth resolves to owner-only. For a real rollout that means: @@ -100,31 +104,41 @@ and adding it to `plugins[]`. Manager visibility is not a feature you can verify on an open-edition checkout; a manager view there shows you your own rows, and that is the edition, not a bug. -### ⛔ Two manager grants are currently narrower than this table implies - -`duly_manager` on `duly_task` and `duly_duty`, and `duly_admin` on `duly_task`, -should read `unit_and_below`. They are authored `own`. - -The reason is not caution. `defineStack` **refuses to load** a permission set -carrying `unit`, `unit_and_below` or `own_and_reports` unless the stack declares -`requires: ['hierarchy-security']` — it is a hard error, not the silent fallback -this repo's own notes describe, and it takes `validate`, `build` and every test -that imports the config. This package may not add that declaration -(`objectstack.config.ts` is off-limits to feature work, and a repo rule forbids -it), so the only authorable depths here are `own` and `org` — and `org` would -hand every manager every task in the tenant. - -`own` is also exactly what an open-edition runtime would have *resolved* -`unit_and_below` to, so nothing about today's behaviour differs. What differs is -that the declaration is now honest, and an enterprise deployment inherits an -under-grant it can see rather than a grant that quietly never worked. - -The three affected grants are recorded machine-readably in -`HIERARCHY_SCOPES_DEFERRED` (`src/security/permission-sets.ts`) and pinned in -both directions by `test/security.test.ts`: widen a grant without deleting its -row and the test fails; delete a row without widening the grant and the test -fails. Tracked as **#46**, which also carries the measurement showing the -"declaring it fails an open-edition boot" belief to be false. +### The declaration, and the warning that is supposed to be there + +`objectstack.config.ts` declares `requires: ['hierarchy-security']`. That is +not optional and it is not a rollout step — **it is what makes the depth scopes +authorable at all**. `defineStack`'s `validateHierarchyScopeCapability` refuses +to load a stack that grants `own_and_reports`, `unit` or `unit_and_below` +without it. That refusal is deliberate: it replaces a silent fail-closed to +owner-only with an authoring-time error, so the metadata cannot claim a depth +the runtime will not enforce (ADR-0049). + +**Declaring it installs nothing.** On this open-edition checkout, with +`@objectstack/security-enterprise` absent, `validate`, `typecheck`, `test` and +`build` all exit 0 and the kernel boots. What you get is one warning on every +validate and build: + +> ⚠ Capability "hierarchy-security" is provided by +> @objectstack/security-enterprise (ADR-0057 hierarchy scopes ship in the +> enterprise edition). Run `pnpm add @objectstack/security-enterprise` and add +> it to `plugins[]`, or remove "hierarchy-security" from `requires`. + +⛔ **That warning is the expected state of this repo — do not silence it.** It +is the honest signal that the depth this package declares is not being enforced +in this checkout. The only two ways to remove it are installing the enterprise +package (deliberately not done here) and deleting the declaration, which puts +the hard load error back and forces every manager grant down to `own`. + +⛔ **`org` is not a hierarchy scope**, so it loads with no declaration at all. +It is the wrong answer for manager visibility and the reason this section is +long: `org` on `duly_task` hands every manager every task in the tenant. +`duly_admin`'s `org` read on `duly_duty` is a deliberate, separate decision +about obligations, not people's work rows. + +`test/security.test.ts` pins both halves — the three depth grants, and the +declaration they require — so neither can be removed without the other going +red. --- @@ -195,6 +209,11 @@ pnpm validate # reports "Security: 3 Positions 3 Permissions" pnpm test # test/security.test.ts asserts every declared scope ``` +`validate` also prints the `hierarchy-security` capability warning on an +open-edition checkout. It is expected — see above. A clean run here means the +enterprise package is installed, not that the warning was a problem someone +fixed. + `test/security.test.ts` asserts the **authored** metadata, never resolved rows — on an open-edition checkout a row count measures the edition, not the declaration, and would go green on the day someone deleted a scope. To see diff --git a/docs/product/data-model.md b/docs/product/data-model.md index e7596ed..340db9e 100644 --- a/docs/product/data-model.md +++ b/docs/product/data-model.md @@ -159,12 +159,23 @@ Every object states `sharingModel` explicitly — unset means private and the publish linter errors on it (ADR-0090 D1/D7). Manager visibility comes from permission sets with `readScope`, not from an -org-wide default. The ADR-0057 depth scopes — `own_and_reports`, `unit`, -`unit_and_below`, `org` — are resolved by `@objectstack/security-enterprise`, -which enterprise deployments carry. We author against them directly and build no -application-level fallback. - -In an open-edition checkout the resolver is absent and those scopes fall back to -owner-only. That is the expected behaviour of this repo, not a defect: a manager -view here shows you your own rows. Anyone verifying manager visibility needs an -enterprise runtime to see it work. +org-wide default. The ADR-0057 hierarchy scopes — `own_and_reports`, `unit`, +`unit_and_below` — are resolved by `@objectstack/security-enterprise`, which +enterprise deployments carry. We author against them directly and build no +application-level fallback. (`org` is a flat scope, not a hierarchy one; it is +used on the catalog and nowhere near a person's rows.) + +Authoring a hierarchy scope **requires** `requires: ['hierarchy-security']` in +`objectstack.config.ts`, which this package declares. Omitting it is not a +silent degradation — `defineStack` refuses to load the stack, which is the +platform deliberately turning the old fail-closed-to-owner-only into an +authoring-time error (ADR-0049: otherwise the metadata would lie). + +Declaring the capability installs nothing and breaks nothing. On an +open-edition checkout, with `@objectstack/security-enterprise` absent, every +gate stays green and `pnpm validate` prints one warning naming the package that +provides the capability. **That warning is the expected state of this repo.** + +The scopes then resolve to owner-only here, so a manager view shows you your own +rows. That is the edition, not a defect; anyone verifying manager visibility +needs an enterprise runtime to see it work. diff --git a/objectstack.config.ts b/objectstack.config.ts index 80e83cf..70fcb46 100644 --- a/objectstack.config.ts +++ b/objectstack.config.ts @@ -60,14 +60,31 @@ export default defineStack({ // `automation` backs flow and job execution, and materialises declarative // `connectors:` entries at boot (ADR-0097). The dispatcher lives there. // - // `hierarchy-security` is deliberately NOT declared here. Manager visibility - // is authored with the ADR-0057 depth scopes ('own_and_reports', 'unit', - // 'unit_and_below', 'org'), which @objectstack/security-enterprise resolves — - // enterprise deployments have it, so no application-level fallback is built. - // This open-edition checkout runs without it and those scopes resolve to - // owner-only; that is the expected open-edition behaviour, not a bug to work - // around. Declaring the capability would make an open-edition boot fail. - requires: ['automation'], + // `hierarchy-security` is REQUIRED, not optional, and declaring it installs + // nothing. Manager visibility is authored with the ADR-0057 depth scopes, + // and `defineStack`'s own `validateHierarchyScopeCapability` refuses to load + // a stack that grants `own_and_reports`, `unit` or `unit_and_below` without + // this entry: + // + // > A stack that uses one MUST declare `requires: ['hierarchy-security']`; + // > otherwise the open runtime would silently fail closed to owner-only + // > (the metadata would lie, ADR-0049). This makes that an authoring-time + // > error instead. + // + // (`org` is NOT a hierarchy scope and passes that check — which is the + // trapdoor: it is authorable without this line and discloses the whole + // tenant. Depth for managers is `unit_and_below`, never `org`.) + // + // Declaring it does NOT fail an open-edition boot. Measured on this + // checkout with @objectstack/security-enterprise absent: `validate`, `test` + // and `build` all exit 0 and the kernel logs `Bootstrap complete`; the + // capability-provider check prints ONE warning naming the package to + // install. That warning is the expected state here — the honest signal that + // these scopes resolve to owner-only until a deployment provides the + // resolver. Do not silence it: the only ways to are installing the + // enterprise package (not wanted in this repo) or deleting this entry, + // which puts the hard error back. + requires: ['automation', 'hierarchy-security'], plugins: [ new ConnectorRestPlugin(), @@ -90,10 +107,13 @@ export default defineStack({ hooks: dulyHooks, functions: dulyFunctions, - // Security posture. Hierarchy read scopes ('own_and_reports', 'unit', - // 'unit_and_below', 'org') are resolved by @objectstack/security-enterprise, - // which is a HARD product dependency — without it they fail closed to - // owner-only, silently. See docs/product/data-model.md#security-posture. + // Security posture. The hierarchy read scopes ('own_and_reports', 'unit', + // 'unit_and_below') are resolved by @objectstack/security-enterprise, which + // is a HARD product dependency — declared as `hierarchy-security` in + // `requires` above. Without the package installed they resolve to + // owner-only, announced by the capability-provider warning on every + // validate/build rather than silently. + // See docs/product/data-model.md#security-posture. positions: dulyPositions, permissions: dulyPermissionSets, sharingRules: dulySharingRules, diff --git a/src/security/index.ts b/src/security/index.ts index e771652..aec607a 100644 --- a/src/security/index.ts +++ b/src/security/index.ts @@ -33,7 +33,6 @@ export { DULY_CATALOG_APPLY, DULY_CATALOG_SYNC, DULY_TASK_UPDATE_STATUS, - HIERARCHY_SCOPES_DEFERRED, } from './permission-sets.js'; export const dulyPositions = [MemberPosition, ManagerPosition, AdminPosition]; diff --git a/src/security/permission-sets.ts b/src/security/permission-sets.ts index 2586c2b..fd517fc 100644 --- a/src/security/permission-sets.ts +++ b/src/security/permission-sets.ts @@ -37,14 +37,22 @@ import type { ObjectPermission } from '@objectstack/spec/security'; * this file is on the READ axis. The write axis never leaves `own` on * `duly_task` and `duly_duty` — see the invariant note on ADMIN_OBJECTS. * - * ⛔ ── The manager's DEPTH is not authorable on this platform version ───── - * The card specifies `readScope: 'unit_and_below'` for the manager grants on - * `duly_task` and `duly_duty`, on the understanding that an open-edition - * runtime resolves it silently to owner-only. It does not: `defineStack` - * REFUSES TO LOAD. See {@link HIERARCHY_SCOPES_DEFERRED} below for the - * measurement and the tracking issue. The three affected grants are authored - * `'own'` — fail-closed — and the intended value is recorded there so it is - * one edit away, and one failing test away from being forgotten. + * ── The manager's DEPTH, and what makes it authorable ──────────────────── + * `readScope: 'unit_and_below'` on the manager grants for `duly_task` and + * `duly_duty` is declared, not deferred. It loads because + * `objectstack.config.ts` declares `requires: ['hierarchy-security']`; + * `defineStack`'s `validateHierarchyScopeCapability` refuses any grant using + * `own_and_reports` / `unit` / `unit_and_below` without it, as an + * AUTHORING-TIME error deliberately put in place of a silent fail-closed to + * owner-only (ADR-0049 — the metadata would otherwise lie). The declaration + * installs nothing: on an open-edition checkout the scopes resolve to + * owner-only and `validate` prints one warning naming + * `@objectstack/security-enterprise`. That warning is the expected state. + * + * ⛔ `org` is NOT a hierarchy scope, so it passes that check with no + * declaration at all. It is the wrong answer here and the reason this note + * exists: `org` on `duly_task` would hand every manager every task in the + * tenant. Depth for managers is `unit_and_below`. * * ── Depth on `duly_catalog_item` ───────────────────────────────────────── * `duly_catalog_item` is `public_read`, so `readScope` there would be inert: @@ -87,62 +95,6 @@ export const DULY_TASK_UPDATE_STATUS = 'duly.task.update_status'; export const DULY_CATALOG_APPLY = 'duly.catalog.apply'; export const DULY_CATALOG_SYNC = 'duly.catalog.sync'; -/** - * ⛔ STOPGAP — the ADR-0057 hierarchy depths this package MEANS to grant and - * may not declare today. Written to be deleted, not maintained. Tracked as - * **#46**, which is where the decision lives; this constant is only the - * machine-readable record of what the compromise costs. - * - * ── The measurement ────────────────────────────────────────────────────── - * `defineStack`'s `validateHierarchyScopeCapability` (@objectstack/spec - * 17.2.0) is a HARD ERROR on any permission-set grant whose `readScope` or - * `writeScope` is `unit`, `unit_and_below` or `own_and_reports`, unless the - * stack declares `requires: ['hierarchy-security']`: - * - * ✗ permission set 'duly_manager' grant on 'duly_task' uses - * readScope='unit_and_below', a HIERARCHY scope. Declare - * `requires: ['hierarchy-security']` (provided by - * @objectstack/security-enterprise) — the open edition cannot enforce it - * and would fail closed to owner-only. - * - * It runs inside `defineStack()`, so it takes `validate`, `build` AND the - * three tests that import the config. This is NOT the "silent fallback to - * owner-only" AGENTS.md rule 7 describes — nothing silent happens; the config - * will not load. - * - * The prescribed fix is one line in `objectstack.config.ts`, which this card - * may not touch and which rule 7 forbids on the grounds that it "would fail an - * open-edition boot". Measured on this checkout, with the capability declared - * and no `@objectstack/security-enterprise` installed, that is not what - * happens: `validate`, `test` (278 passing, kernel logged - * `✅ Bootstrap complete`) and `build` all exit 0, with one warning naming the - * missing provider. The full measurement is in #46. Correcting a rule written - * in four places, in the config every parallel task shares, is not a rider on - * a security PR — hence the issue rather than the edit. - * - * ── Why `own` and not `org` ────────────────────────────────────────────── - * `org` IS authorable (it is not a hierarchy scope), and it is the wrong - * answer: it would hand every manager every task and duty in the tenant. - * Under-granting is visible and fixable; over-granting is neither. The three - * entries below are authored `own` — which is also exactly what an - * open-edition runtime would have RESOLVED `unit_and_below` to, so nothing - * about the running behaviour changes here. What changes is that the - * declaration is now honest, and an enterprise deployment inherits an - * under-grant it can see rather than a grant that quietly never worked. - * - * ── How this gets undone ───────────────────────────────────────────────── - * `test/security.test.ts` pins every row: the authored value must equal - * `authored`, and `intended` must be a hierarchy scope the validator would - * reject today. Widen a grant without deleting its row and the test fails; - * delete a row without widening the grant and the test fails. The compromise - * cannot outlive its reason, and it cannot be half-undone. - */ -export const HIERARCHY_SCOPES_DEFERRED = { - 'duly_manager.duly_task.readScope': { authored: 'own', intended: 'unit_and_below' }, - 'duly_manager.duly_duty.readScope': { authored: 'own', intended: 'unit_and_below' }, - 'duly_admin.duly_task.readScope': { authored: 'own', intended: 'unit_and_below' }, -} as const; - /** * ⛔ `duly_log_entry` — the entry that must never be widened. * @@ -214,20 +166,34 @@ const MEMBER_OBJECTS = { } satisfies Record; /** - * Manager = member, plus the one write a manager makes. + * Manager = member, plus depth on the read axis and the one write a manager + * makes. * - * ⛔ `duly_task` and `duly_duty` are INHERITED UNCHANGED, which is not what - * the card asked for: both should widen to `readScope: 'unit_and_below'` on - * the read axis. That value will not load — see - * {@link HIERARCHY_SCOPES_DEFERRED}. What survives is the half that - * was never in doubt: `writeScope` stays `'own'` on both, inherited rather - * than restated, so the "a manager writes nothing below them" invariant is - * held by the absence of an override rather than by remembering to retype the - * same value. + * ⛔ Both depth overrides below change `readScope` ONLY. Each spreads the + * member entry, so `writeScope: 'own'` is inherited rather than retyped — the + * "a manager writes nothing below them" invariant is held by the absence of + * an override, not by remembering to repeat a value. Widening either of these + * to a write depth is the one edit `test/security.test.ts` will not let + * through. */ const MANAGER_OBJECTS = { ...MEMBER_OBJECTS, + // Reads down the unit and everything under it (ADR-0057). This is the + // manager's whole job: answer "what is late across my unit" having entered + // no data. Write depth is inherited `own` — untouched, deliberately. + duly_task: { + ...MEMBER_OBJECTS.duly_task, + readScope: 'unit_and_below', + }, + + // Same depth on the obligations behind those tasks: a manager who can see a + // late task and not the duty that produced it cannot act on it. + duly_duty: { + ...MEMBER_OBJECTS.duly_duty, + readScope: 'unit_and_below', + }, + // Assigning is a manager's only write. Create and edit their own // assignments; the fan-out then produces one independently-owned task per // assignee, which is where status entry lives. @@ -257,6 +223,11 @@ const MANAGER_OBJECTS = { * what it touched. A correction that goes through it is auditable; a * correction typed into somebody's duty record is not. * + * `duly_task` is inherited untouched and therefore reads `unit_and_below` — + * the administrator's depth on tasks arrives through the manager set rather + * than being restated here, which is why widening it is a one-line edit in + * one place. + * * `duly_log_entry` is inherited untouched. There is no admin override, and * that is the whole point of the invariant. */ diff --git a/test/security.test.ts b/test/security.test.ts index d026147..8fec595 100644 --- a/test/security.test.ts +++ b/test/security.test.ts @@ -10,7 +10,6 @@ import { DULY_CATALOG_APPLY, DULY_CATALOG_SYNC, DULY_TASK_UPDATE_STATUS, - HIERARCHY_SCOPES_DEFERRED, ManagerPermissionSet, ManagerPosition, MemberPermissionSet, @@ -20,6 +19,7 @@ import { dulySharingRules, } from '../src/security/index.js'; import { dulyActions } from '../src/actions/index.js'; +import stack from '../objectstack.config.js'; import { CatalogItem, Duty, LogEntry, Task, Assignment } from '../src/objects/index.js'; /** @@ -103,17 +103,18 @@ describe('permission sets — declared scopes, object by object', () => { duly_assignment: { read: 'own', write: 'own' }, }, duly_manager: { - // ⛔ `duly_task` / `duly_duty` read SHOULD be 'unit_and_below'. It is not - // authorable on protocol 17.2.0 — see HIERARCHY_SCOPES_DEFERRED and the - // dedicated block below, which pins the deferral itself. - duly_task: { read: 'own', write: 'own' }, - duly_duty: { read: 'own', write: 'own' }, + // The two ADR-0057 depth grants. Authorable because the stack declares + // `requires: ['hierarchy-security']`; the dedicated block below pins + // that dependency in both directions. + duly_task: { read: 'unit_and_below', write: 'own' }, + duly_duty: { read: 'unit_and_below', write: 'own' }, duly_log_entry: { read: 'own', write: 'own' }, duly_catalog_item: { read: 'own', write: 'own' }, duly_assignment: { read: 'own', write: 'own' }, }, duly_admin: { - duly_task: { read: 'own', write: 'own' }, + // Inherited from the manager set, not restated. + duly_task: { read: 'unit_and_below', write: 'own' }, // Org-wide READ of duties; the write axis stays at the baseline. duly_duty: { read: 'org', write: 'own' }, duly_log_entry: { read: 'own', write: 'own' }, @@ -249,7 +250,7 @@ describe('no write scope on duly_task / duly_duty is wider than own', () => { * maps were built by spreading it. */ describe('manager and admin inherit rather than restate', () => { - const MANAGER_OVERRIDES = ['duly_assignment']; + const MANAGER_OVERRIDES = ['duly_task', 'duly_duty', 'duly_assignment']; const ADMIN_OVERRIDES = ['duly_catalog_item', 'duly_duty']; it('every non-overridden manager entry is the member entry itself', () => { @@ -270,6 +271,24 @@ describe('manager and admin inherit rather than restate', () => { } }); + it('the manager depth overrides keep every member key they do not change', () => { + // The failure this catches is the one this card could most easily have + // caused: retyping the entry to add `readScope` and dropping + // `writeScope: 'own'` on the way, which widens the write axis by omission. + for (const objectName of ['duly_task', 'duly_duty']) { + const base = MemberPermissionSet.objects[objectName]; + const override = ManagerPermissionSet.objects[objectName]; + for (const key of Object.keys(base) as Array) { + if (key === 'readScope') continue; // the one deliberate widening + expect(override[key], `manager.${objectName} dropped ${String(key)}`).toEqual(base[key]); + } + expect(override.readScope, `manager.${objectName} reads its unit and below`).toBe( + 'unit_and_below', + ); + expect(override.writeScope, `manager.${objectName} write depth must stay own`).toBe('own'); + } + }); + it('an override keeps every base key it does not change', () => { // The failure this catches: rewriting an entry from scratch and dropping // `writeScope: 'own'` on the way, which widens by omission. @@ -377,42 +396,122 @@ describe('action capability gates', () => { }); /** - * ⛔ STOPGAP — delete this block together with `HIERARCHY_SCOPES_DEFERRED`. + * ⛔ The declaration the depth grants stand on, pinned in both directions. + * + * `readScope: 'unit_and_below'` is authorable here only because + * `objectstack.config.ts` declares `requires: ['hierarchy-security']`. + * `defineStack`'s `validateHierarchyScopeCapability` refuses to load a stack + * that grants `own_and_reports` / `unit` / `unit_and_below` without it — an + * AUTHORING-TIME hard error, deliberately in place of a silent fail-closed to + * owner-only (ADR-0049: the metadata would otherwise lie). Declaring it + * installs nothing and fails nothing: on this open-edition checkout every gate + * stays green and `validate` prints one warning naming the provider package. + * The measurement is in duly#46. * - * It pins the compromise in both directions so it cannot rot: widen a grant - * without deleting its row and the "authored" assertion fails; delete a row - * without widening the grant and the coverage assertion fails. See duly#46. + * Neither half can be deleted alone: + * · change a depth grant and the per-grant assertion fails; + * · author a hierarchy scope anywhere without the declaration and the + * implication below fails HERE — a named test rather than an opaque + * config-load failure taking out every suite that imports the config. */ -describe('deferred hierarchy scopes (stopgap — see #46)', () => { +describe('hierarchy depth grants and the capability they require', () => { + const HIERARCHY_SCOPES: readonly string[] = ['own_and_reports', 'unit', 'unit_and_below']; + + /** Every (set, object, axis) in this package whose declared scope needs the resolver. */ + const authoredHierarchyScopes = ALL_SETS.flatMap((set) => + Object.entries(set.objects).flatMap(([objectName, grant]) => + (['readScope', 'writeScope'] as const) + .filter((axis) => HIERARCHY_SCOPES.includes(scopeOf(grant[axis]))) + .map((axis) => `${set.name}.${objectName}.${axis}`), + ), + ); + + /** The manager model, machine-readably: what depth exists, and nothing else. */ + const DEPTH_GRANTS = [ + 'duly_admin.duly_task.readScope', + 'duly_manager.duly_duty.readScope', + 'duly_manager.duly_task.readScope', + ]; + const setsByName = new Map(dulyPermissionSets.map((s) => [s.name, s])); - for (const [key, record] of Object.entries(HIERARCHY_SCOPES_DEFERRED)) { + for (const key of DEPTH_GRANTS) { const [setName, objectName, axis] = key.split('.'); - it(`${key} is authored '${record.authored}', not the intended '${record.intended}'`, () => { + it(`${key} is 'unit_and_below'`, () => { const grant = setsByName.get(setName)?.objects[objectName]; expect(grant, `${setName} must grant ${objectName}`).toBeDefined(); - expect(scopeOf(grant?.[axis as 'readScope' | 'writeScope'])).toBe(record.authored); - - // The intent must still be a scope `validateHierarchyScopeCapability` - // rejects. If it stops being one, the deferral has no reason to exist. - expect(['own_and_reports', 'unit', 'unit_and_below']).toContain(record.intended); - // And it must be a WIDENING — a deferral that narrows is a mistake. - expect(SCOPE_WIDTH[record.intended]).toBeGreaterThan(SCOPE_WIDTH[record.authored]); + expect(scopeOf(grant?.[axis as 'readScope' | 'writeScope'])).toBe('unit_and_below'); }); } - it('records every grant the card asked to widen, and no others', () => { - expect(Object.keys(HIERARCHY_SCOPES_DEFERRED).sort()).toEqual([ - 'duly_admin.duly_task.readScope', - 'duly_manager.duly_duty.readScope', - 'duly_manager.duly_task.readScope', - ]); + it('those three are the only hierarchy scopes authored anywhere', () => { + // Also what keeps the implication below from going vacuous. + expect([...authoredHierarchyScopes].sort()).toEqual(DEPTH_GRANTS); + }); + + it('the stack declares hierarchy-security, as every scope above requires', () => { + expect(authoredHierarchyScopes.length).toBeGreaterThan(0); + expect(stack.requires ?? []).toContain('hierarchy-security'); + }); + + it('no set reaches for `org` on duly_task — the trapdoor next to this one', () => { + // `org` is NOT a hierarchy scope, so it loads with no declaration at all. + // That makes it the nearest wrong answer for anyone who wants "some + // visibility for managers": it discloses every task in the tenant. + for (const set of ALL_SETS) { + expect(scopeOf(set.objects.duly_task?.readScope), `${set.name} reads all tasks`).not.toBe( + 'org', + ); + } }); +}); + +/** + * The write axis, across every set and every object. + * + * "A manager reads down and writes nothing but their own" is the product, and + * this is where it stops being an intention. Two assertions, because they fail + * for different reasons: the first catches any widening at all, the second + * catches the specific one this card could have introduced — depth leaking + * from the read axis onto the write axis. + */ +describe('no write scope is wider than own', () => { + /** + * The one exception, declared rather than tolerated. `duly_catalog_item` is + * `public_read` — read-open but WRITE-OWNED — so without an org write depth + * an administrator could only edit catalog items they personally created. + * The catalog describes positions, not people: there is no personal record + * behind this entry. Anything else appearing here is a regression, and the + * equality below is what makes adding a second one a deliberate act. + */ + const DECLARED_EXCEPTIONS: Record = { + 'duly_admin.duly_catalog_item': 'org', + }; - it('never defers a WRITE axis — that one is an invariant, not a compromise', () => { - for (const key of Object.keys(HIERARCHY_SCOPES_DEFERRED)) { - expect(key.endsWith('.readScope'), `${key} must not defer a write scope`).toBe(true); + it('every write scope in the package is own, bar the one declared exception', () => { + const wider: Record = {}; + for (const set of ALL_SETS) { + for (const [objectName, grant] of Object.entries(set.objects)) { + const scope = scopeOf(grant.writeScope); + if (SCOPE_WIDTH[scope] > SCOPE_WIDTH.own) wider[`${set.name}.${objectName}`] = scope; + } + } + expect(wider).toEqual(DECLARED_EXCEPTIONS); + }); + + it('no write scope anywhere is a HIERARCHY scope — depth never reaches the write axis', () => { + // Unconditional, exception included: `org` is a flat grant on one + // reference table, whereas a write depth would mean one person writing + // another person's rows by virtue of where they sit in the tree. That is + // the thing the product does not do, on any object. + for (const set of ALL_SETS) { + for (const [objectName, grant] of Object.entries(set.objects)) { + expect( + ['own_and_reports', 'unit', 'unit_and_below'], + `${set.name}.${objectName} writes by hierarchy depth`, + ).not.toContain(scopeOf(grant.writeScope)); + } } }); });