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
59 changes: 59 additions & 0 deletions .changeset/16678-admin-set-user-manager.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
'@objectstack/plugin-auth': patch
'@objectstack/lint': patch
---

`sys_user.manager_id` gains an admin write surface: `POST /api/v1/auth/admin/set-user-manager`

`{ type: 'manager' }` is the canonical first rung of a tiered approval ladder,
and it resolves `sys_user.manager_id` — a column **no product surface could
write**. Measured: the generic data path refuses it (the ADR-0092 D2
managed-update whitelist for `sys_user` is `{name, image, locale}`), the admin
bulk import does not carry it (`admin-import-users.ts` matches `manager_id` 0
times, against a control of `phone_number` 8), and the column is `readonly` on
the user form. So on any install without a directory sync the rung expanded to
nobody, the request opened on a slate no one could act on, and under the
default `lockRecord: true` the record stayed locked.

**The endpoint.** A platform admin posts `{ userId, managerId }`; `managerId:
null` clears the link. It is an ObjectStack mount on the raw app ahead of the
better-auth catch-all — the same family as `POST /api/v1/auth/admin/unlock-user`
— platform-admin gated (ADR-0068) and ledgered in `auth-route-ledger.ts`.

**It is not a new editable profile column, and that is the design.** The
handler runs under a **system context**, so it reaches the column by context
rather than by a whitelist entry — the same way `admin-import-users` already
reaches `phone_number` and `role`. `SYS_USER_PROFILE_EDIT_FIELDS` is
untouched, `MANAGED_EXTENSION_EDITABLE_FIELDS.sys_user` stays `{locale}`, and
`sys_user.manager_id` keeps `readonly: true`, so ADR-0092 D4 still holds by
construction. Since ADR-0092 D5's amendment made Tier-1 membership imply
self-editability, admitting the column to Tier 1 would have handed every member
their own first-rung approver and a widening of their own `own_and_reports`
read scope; it is not admitted.

**Five refusals, every one enforced at the write** — the only manager-chain
walkers in the open tree are single-hop, so nothing downstream catches a bad
link: self-assignment; a link that closes a cycle (the walk is itself
cycle-safe, so a pre-existing loop is reported rather than hung on); a chain
past the depth cap that ADR-0057 D3's bounded rollups require; a manager
provably outside every organization the user belongs to (beside, not instead
of, the existing routing-time screen); and any identity whose `sys_user.source`
is `idp_provisioned`, where the directory stays the one authoring surface.

**`@objectstack/lint`** keeps the `approval-approvers-may-resolve-empty`
advisory and its `stackWiresManagerChain` silencer — the dead end it reports
survives the write surface, because a static check still cannot read the
column; only its *cause* became recoverable. What changed is the remedy text,
which named a column with no route and now names the endpoint, its body, how to
clear the link, and what it refuses. The Approvals guide carries the same
rewrite in prose.

**Why `patch` and not `minor`.** No new exported symbol is reachable from
either published entry: `admin-set-user-manager.ts` is deliberately not
re-exported from `plugin-auth/src/index.ts` and is not named in the package's
`exports` map, so none of `runSetUserManager`, `MAX_MANAGER_CHAIN_DEPTH`,
`SetUserManagerDeps`, `SetUserManagerEngine`, `SetUserManagerResult` or
`SetUserManagerRefusalReason` appears in the built `dist/index.d.ts`. No
already-published payload gains a key — the endpoint's response is a new
payload, not a new field on an old one. A new **route** is wire, and wire
compatibility is not the grading floor.
50 changes: 32 additions & 18 deletions content/docs/automation/approvals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,40 @@ Authored `type: 'role'` on 15.x? That is the deprecated spelling of `org_members
</Callout>

<Callout type="warn">
**`manager` is a directory-sync dependency, not something you configure here.**
`{ type: 'manager' }` resolves at runtime from `sys_user.manager_id` on the record's owner, and
that column is not one an operator can fill in from inside the product.

`sys_user.manager_id` has no product write surface — the data API's managed-update whitelist is
`{name, image, locale}`, the auth admin endpoints do not accept the column and the Console
renders no field for it, so it is never populated by editing the user in the Console. On this
platform the column is written by a seed, or by any other system-context write, which bypasses
the managed-update whitelist. SCIM provisioning and directory sync can populate it too, but only
**`manager` is set on the user by an admin, not configured here.**
`{ type: 'manager' }` resolves at runtime from `sys_user.manager_id` on the record's owner. That
column is not a profile field: it is `readonly` on the user form and the data API's
managed-update whitelist is `{name, image, locale}`, so it is never populated by editing the user
in the Console.

It has a dedicated admin operation instead. A platform admin posts the link:

```http
POST /api/v1/auth/admin/set-user-manager
Content-Type: application/json

{ "userId": "usr_report", "managerId": "usr_boss" }
```

`"managerId": null` clears the link. The endpoint refuses a link that would make a user their own
manager, close a cycle in the reporting chain, run past the chain depth cap, or point at a manager
who is provably outside every organization the user belongs to — each with the reason in the
response. It is platform-admin gated, like every other `/api/v1/auth/admin/*` route.

The column is also written by a seed, or by any other system-context write, which bypasses the
managed-update whitelist. SCIM provisioning and directory sync can populate it too, but only
through a provisioning path your own deployment supplies: this platform declares the SCIM
`manager` attribute without projecting it onto the column, and its admin bulk import does not
write it either.

So where `manager_id` is unset the slate resolves to nobody: the request opens with an empty
`pending_approvers` and waits forever, and under the default `lockRecord: true` the record stays
locked with no in-product recovery. Populate the column for everyone who submits this request,
or add a fallback approver that cannot resolve empty — e.g.
`{ type: 'org_membership_level', value: 'owner' }`. `os lint` reports this at authoring time
(`approval-approvers-may-resolve-empty`, `info`) and carries the same remedy text, so the two
cannot drift apart.
write it either. Where an identity carries `source: 'idp_provisioned'` the admin operation
**refuses**, so that directory stays the one surface that authors its manager.

Setting the column is still something you have to *do*. Where `manager_id` is unset the rung
expands to nobody, the request opens on a slate no one can act on, and under the default
`lockRecord: true` the record stays locked — recoverable only by a platform or tenant admin
override. So set it for everyone who submits this request, or add a fallback approver that cannot
resolve empty — e.g. `{ type: 'org_membership_level', value: 'owner' }`. `os lint` reports the
shape at authoring time (`approval-approvers-may-resolve-empty`, `info`) and carries the same
remedy text, so the two cannot drift apart.
</Callout>

Approving is itself a gated action — model "may approve" as a capability (`approve_invoice`) the approver's permission set grants, and gate the approve action's `requiredPermissions` on it so the gate is enforced on **both** the UI and the server (ADR-0066 D4).
Expand Down
34 changes: 17 additions & 17 deletions content/docs/permissions/tenant-audit-census.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way.

The same holds twice over for the context. An options argument spelled as a
literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
forwarding shim cannot, and **67 of the 222 sites are spelled that way**. A
forwarding shim cannot, and **67 of the 223 sites are spelled that way**. A
context resolved from an inline literal or a local `const` can be tested for
`isSystem`; one arriving from a helper call cannot.

Expand Down Expand Up @@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page:

| carried figure | where it survives | this census |
| :--- | :--- | ---: |
| 175 write call sites | quoted in the merged changeset | **222** |
| 175 write call sites | quoted in the merged changeset | **223** |
| 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable |
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **148 of 222** decidable, **74** undecidable |
| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 104 decidably elevated, 0 decidably not, 101 undecidable |
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **149 of 223** decidable, **74** undecidable |
| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 104 decidably elevated, 0 decidably not, 102 undecidable |
| 141 and 132, two independent re-derivations | the card that filed this work | — |

**The differences are not reconciled, and deliberately so.** The old census's
Expand All @@ -167,11 +167,11 @@ would report a smaller number and would not say so.

The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
figure has no surviving corroboration anywhere in the tree.** This census reads
104 of 222 (47%) as decidably elevated, with 101 more whose elevation is a
104 of 223 (47%) as decidably elevated, with 102 more whose elevation is a
run-time fact — so the claim is neither confirmed nor refuted, and the honest
answer is that a static reading cannot settle it.

⇒ **Cite `9 / 222`, and say what it is**: the sites whose options argument was
⇒ **Cite `9 / 223`, and say what it is**: the sites whose options argument was
READ and holds no tenant context, against a decidably tenancy-enabled object.
That is the control's provable yield surface. ⛔ Do not cite it as "the sites
without tenant context" — **32 further sites** have an options argument this
Expand All @@ -183,28 +183,28 @@ cannot read, and they are neither in nor out.

| what | count |
| :--- | ---: |
| write call sites on the application surface | **222** |
| …whose object name is statically decidable | 148 |
| write call sites on the application surface | **223** |
| …whose object name is statically decidable | 149 |
| …whose object name is chosen at run time | 74 |
| …against an object with tenancy ENABLED | 148 |
| …against an object with tenancy ENABLED | 149 |
| …against an object that declares tenancy off | 0 |
| threading a tenant context | 138 |
| threading a tenant context | 139 |
| PROVABLY carrying none (options read, no context key) | **17** |
| …of those, against a decidably tenancy-enabled object | **9** |
| options argument UNREADABLE — may or may not carry one | 67 |
| …of those, against a decidably tenancy-enabled object | 32 |
| threading a decidably ELEVATED (`isSystem`) context | 104 |
| threading a context that is decidably NOT elevated | 0 |
| threading a context whose elevation is a run-time fact | 101 |
| threading a context whose elevation is a run-time fact | 102 |

| how the instrument reached the site | count |
| :--- | ---: |
| receiver carried a readable engine type | 178 |
| receiver carried a readable engine type | 179 |
| receiver erased, placed by the object NAME | 18 |
| receiver erased, placed by an `object: string` PARAMETER | 15 |
| receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 |

| object name spelled inline | 108 |
| object name spelled inline | 109 |
| object name spelled through a `const` | 40 |
| object name is an `object: string` parameter | 19 |
| object name is some other run-time expression | 55 |
Expand All @@ -224,13 +224,13 @@ holds still. They are required to be HERE and to say WHEN they were true;
their values are not compared. The reasoning, and the measurement behind it,
are in `scripts/check-tenant-audit-census.mjs`.

Measured on 2026-09-12 at `bbeb190f6`.
Measured on 2026-09-13 at `f62bc2d0a`.

| corpus scale (not enforced) | count |
| :--- | ---: |
| tracked non-test sources scanned | 564 |
| engine-shaped types recognised | 59 |
| tracked non-test sources scanned | 567 |
| engine-shaped types recognised | 61 |
| declared objects in the registry | 300 |
| same-named calls subtracted as non-engine | 137 |
| same-named calls subtracted as non-engine | 138 |

{/* END GENERATED: tenant-audit-census */}
19 changes: 10 additions & 9 deletions docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.

| Measure | Value |
|---|---:|
| Write call sites | 222 |
| Object name statically decidable | 148 |
| Write call sites | 223 |
| Object name statically decidable | 149 |
| Object name chosen at run time | 74 |
| Against a tenancy-enabled object | 148 |
| Against a tenancy-enabled object | 149 |
| Against an object declaring tenancy off | 0 |
| Threading a tenant context | 138 |
| Threading a tenant context | 139 |
| Provably carrying none | 17 |
| …and decidably tenancy-enabled | 9 |
| Options argument unreadable | 67 |
| …and decidably tenancy-enabled | 32 |
| Threading a decidably elevated context | 104 |
| Threading a decidably non-elevated context | 0 |
| Threading a context of undecidable elevation | 101 |
| Threading a context of undecidable elevation | 102 |

## Corpus scale — present and dated, ⛔ NOT enforced

Expand All @@ -52,14 +52,14 @@ holds still. They are required to be HERE and to say WHEN they were true;
their values are not compared. The reasoning, and the measurement behind it,
are in `scripts/check-tenant-audit-census.mjs`.

Measured on 2026-09-12 at `bbeb190f6`.
Measured on 2026-09-13 at `f62bc2d0a`.

| corpus scale (not enforced) | count |
| :--- | ---: |
| tracked non-test sources scanned | 564 |
| engine-shaped types recognised | 59 |
| tracked non-test sources scanned | 567 |
| engine-shaped types recognised | 61 |
| declared objects in the registry | 300 |
| same-named calls subtracted as non-engine | 137 |
| same-named calls subtracted as non-engine | 138 |

## Every site

Expand All @@ -78,6 +78,7 @@ Measured on 2026-09-12 at `bbeb190f6`.
| `packages/plugins/plugin-audit/src/read-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 |
| `packages/plugins/plugin-auth/src/admin-import-users.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 |
| `packages/plugins/plugin-auth/src/admin-import-users.ts` | `update` | `sys_user` | enabled | options unreadable | 2 |
| `packages/plugins/plugin-auth/src/admin-set-user-manager.ts` | `update` | `sys_user` | enabled | context, elevation undecidable | 1 |
| `packages/plugins/plugin-auth/src/admin-user-endpoints.ts` | `insert` | `sys_audit_log` | enabled | elevated | 1 |
| `packages/plugins/plugin-auth/src/admin-user-endpoints.ts` | `update` | `sys_user` | enabled | elevated | 1 |
| `packages/plugins/plugin-auth/src/adopt-membership.ts` | `update` | `SystemObjectName.MEMBER` | undecidable | PROVABLY NONE | 1 |
Expand Down
49 changes: 36 additions & 13 deletions packages/lint/src/validate-approval-approvers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,30 +259,50 @@ describe('unset-manager dead-end (#16748)', () => {
expect(findings[0].message).toContain('locked'); // lockRecord defaults true
});

it('names the REAL remedy — provisioning, not the Console', () => {
it('names the REAL remedy — the admin operation, not the Console form', () => {
const [finding] = validateApprovalApprovers(managerOnly());
// The prescription an operator can actually carry out (#16678: the column
// has no product write surface).
// The prescription an operator can actually carry out. #16678 landed the
// write surface, so the first thing the hint owes the reader is the
// endpoint, its body and how to CLEAR the link.
expect(finding.hint).toContain('/api/v1/auth/admin/set-user-manager');
expect(finding.hint).toContain('{ userId, managerId }');
expect(finding.hint).toContain('managerId set to null');
expect(finding.hint).toContain('SCIM');
expect(finding.hint).toContain('import');
expect(finding.hint).toContain('directory sync');
expect(finding.hint).toContain('no product write surface');
// ⛔ And it must not send them to a surface that cannot write it. The word
// "Console" appears only inside that denial, never as an instruction.

// ⛔ The assertion that went stale the day the endpoint landed. It must be
// gone from the string, not merely contradicted further down it: an author
// who reads "no product write surface" stops looking for the endpoint.
expect(finding.hint).not.toContain('no product write surface');

// ⛔ And it still must not send them to a surface that cannot write it —
// the column stays OUT of ADR-0092 Tier 1 and readonly on the form, so a
// dedicated operation is not the same thing as an editable profile column.
// The word "Console" appears only inside that denial, never as an
// instruction.
expect(finding.hint).toContain('never populated by editing the user in the Console');
expect(finding.hint).not.toMatch(/[Ee]dit .{0,40}in the Console\b(?!.*NOT)/);
// It still offers the escape that does not depend on #16678 at all.
// It still offers the escape that needs no operator action at all.
expect(finding.hint).toContain("org_membership_level', value: 'owner'");
});

it('GRADES the routes — an exact diagnosis whose remedy cannot be carried out is worse than none', () => {
// A remedy that names a route with no writer is the #17037 shape. The three
// routes are measured against this tree, so the hint must SEPARATE the one
// that works here from the ones that need the deployment's own provisioning.
// A remedy that names a route with no writer is the #17037 shape. The four
// routes are measured against this tree, so the hint must SEPARATE the ones
// that work here from the ones that need the deployment's own provisioning.
const [finding] = validateApprovalApprovers(managerOnly());

// The route with a demonstrated writer: a system-context write bypasses the
// managed-update whitelist (`isUserContextWrite` is `userId && !isSystem`).
// The route added by #16678, and the refusals that make it safe to point an
// operator at: naming an endpoint without naming what it declines is the
// same "prescription that cannot be carried out" defect one layer in.
expect(finding.hint).toContain('their own manager');
expect(finding.hint).toContain('close a cycle');
expect(finding.hint).toContain('organization boundary');

// The pre-existing route with a demonstrated writer: a system-context write
// bypasses the managed-update whitelist (`isUserContextWrite` is
// `userId && !isSystem`). Kept — the endpoint did not replace it.
expect(finding.hint).toContain('written by a seed, or by any other system-context write');
expect(finding.hint).toContain('bypasses the managed-update whitelist');

Expand All @@ -294,8 +314,11 @@ describe('unset-manager dead-end (#16748)', () => {

// ⛔ And they must not be deleted: a deployment running a real directory
// sync may well populate the column, and the defect was presenting all
// three as equally available, never naming them at all.
// three as equally available, never naming them at all. They now also take
// PRECEDENCE — the endpoint refuses an `idp_provisioned` identity — so the
// hint must not read as "use the endpoint instead of your directory".
expect(finding.hint).toContain('SCIM provisioning and directory sync can populate it');
expect(finding.hint).toContain("source 'idp_provisioned' the admin operation refuses");
});

it('does not claim a runtime fact it did not read', () => {
Expand Down
Loading
Loading