You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 04669a4
Browse filesBrowse the repository at this point in the historyBrowse files
fix(objectql): a runAs:'user' hook sees its organization's members in sys_user, as the user's own request does (#24518)
Fixes#24497
Clause-②: no (a runtime producer fix: a `runAs: 'user'` hook reads what
the same user's own request reads. Nothing `@objectstack/spec` declares
moves; owes the seat's review of record before the queue)
## What was wrong
`ScopedContext.withRunAs('user')` pins a hook's `ctx.api` to `{
...triggering context, isSystem: false }` and seeds nothing. A hook
fired from a hand-built `{ isSystem: true, userId, tenantId }` context
therefore carried no `org_user_ids`. The `sys_user_org_members` policy
(`id in current_user.org_user_ids`) compiled `unresolved-variable` and
dropped out, so the hook saw the acting user's own `sys_user` row only.
The same user's request saw their organisation's members too. This is
the hook-side twin of #24448 (PR #24495), and a precondition of #24341:
under its one-policy `sys_user` default, such a hook would lose its own
row as well.
## Measurement (real stack, before and after)
The probe is
`packages/services/service-automation/src/runas-user-hook-org-peers.integration.test.ts`.
It boots ObjectKernel, ObjectQLPlugin, driver-sql on better-sqlite3
`:memory:`, the identity objects (`createIdentityObjectsPlugin`) and the
real SecurityPlugin with its shipped default permission sets. A `runAs:
'user'` hook on `hook_probe`'s `afterInsert` reads `sys_user` through
`ctx.api`, and a middleware records the context that read carries.
Fixture: organisation A (the Default Organization) and organisation B.
`usr_me` belongs to A and B and acts in A. `usr_peer` belongs to A.
`usr_other` belongs to B only. `usr_loner` belongs to none.
The probe was committed alone first (`1f1afac08`) and run against
unchanged `origin/main` `666283aea`. The after column is head
`e82725a18`.
| triggering context | before: rows / hook context | after: rows / hook
context |
|:--|:--|:--|
| `{ isSystem: true, userId: usr_me, tenantId: A }` (the card's shape) |
`usr_me` / no positions, no permissions, no `org_user_ids` | `usr_me,
usr_peer` / positions `org_member, everyone`, permissions
`member_default`, `org_user_ids` `usr_me, usr_peer` |
| the request-resolved envelope (control) | `usr_me, usr_peer` /
envelope verbatim | `usr_me, usr_peer` / envelope verbatim, nothing
resolved |
| approvals' exact envelope `{ isSystem: true, positions: [],
permissions: [], userId: usr_me }` | `usr_me` / `org_user_ids` absent
(read from the ablation leg) | `usr_me` / `org_user_ids` `usr_me`,
positions and permissions `[]` verbatim |
| no organisation `{ isSystem: true, userId: usr_me }` | `usr_me` |
`usr_me` / `org_user_ids` `usr_me` |
`usr_other` and `usr_loner` are never in any answer. The CONTROL case,
the user's own request acting in A, answers `usr_me, usr_peer` on both
trees.
The probe does not fire through `plugin-approvals`' real write path. No
package composes approvals, the SecurityPlugin and the identity objects
in its dev dependencies. `plugin-approvals` lacks `plugin-security` and
`plugin-auth`; `service-automation` and `runtime` lack
`plugin-approvals`; `qa/dogfood` and `cli` have all three but boot the
full app. Adding a dev dependency would move the lockfile outside this
card's fence. So the probe fires from the approvals' exact envelope,
copied from `SYSTEM_CTX` in `approval-service.ts` and
`approval-node.ts`.
## The change
- **`ScopedContext[SEED_USER_GRANTS]()` (`engine.ts`, the
`ScopedContext` class)**, keyed on a symbol declared in `hook-run-as.ts`
and not exported from the package entry, is an async method, the second
half of the `'user'` derivation. It applies #24495's run-setup rule to
the hook's pinned context:
- Seeding uses `resolveUserAuthzGrants` (`@objectstack/core`), the
resolver the request path seeds through. There is no second resolver and
no second definition of "peer".
- A context with no `permissions` takes the resolved positions and
permission sets.
- Positions and permission sets the context already carries stay
verbatim, and only the peer list is taken, so a narrowed ceiling is
never re-broadened.
- A context that already carries `org_user_ids` resolves nothing.
- It reads no elevation flag: it runs only on the api
`withRunAs('user')` produced.
- **`seedHookUserGrants` (`hook-run-as.ts`)** is the step the wrapper
awaits after `deriveHookApi`. An api that cannot seed, such as the
refusing `UnscopedHookApi`, is returned as is.
- **`installRunAsApi` (`hook-wrappers.ts`)** is now async for `'user'`
only.
- `'inherit'` takes no extra `await` and keeps the engine-built api by
reference.
- `'system'` installs `sudo()` as before.
- The `UnscopedHookApi` refusal (no `userId`) does not move.
- When the resolver fails (an unreadable permission store), the hook
keeps the unseeded pin and one `warn` names the consequence. This is
narrower, never elevated, and the same posture as the flow run setup.
**Why `withRunAs` stays synchronous.** `runtime`'s stored-metadata seams
(`deriveThroughSeam`) wrap `withRunAs`'s return value synchronously. A
Promise there would hand back an unwrapped api, which is a route around
the seam. So the async half is a separate method, reached by the wrapper
while `ctx.api` is still the engine-built `ScopedContext`. The sandbox
body layers wrap it later, at handler time.
**Cost.** A `runAs: 'user'` hook whose triggering context lacks
`permissions` or `org_user_ids` pays one `resolveUserAuthzGrants` per
dispatch: one wave of five concurrent reads, plus the permission-set
reads the resolver makes. A request-resolved context carries both and
pays nothing. The request-grants memo does not apply here, because it is
open only inside `resolveAuthzContext`. The #11971 grants cache answers
it when `OS_AUTHZ_GRANTS_CACHE_TTL_MS` is enabled; it is off by default.
A hook is request-shaped, so it is not added to the cache's ruled bypass
list.
**Where the files landed vs the claim's fence.** The three source files
are in the fence. Outside it:
- `engine.ts` gains one import line for `resolveUserAuthzGrants` beside
its existing `@objectstack/core` imports. That is not PR #24508's
predicate region.
- The real-stack probe lives in `service-automation`, beside its flow
twin. That package is the one whose dev dependencies already compose the
whole stack; `objectql` cannot depend on driver-sql, plugin-auth or
plugin-security.
## Pins
- `hook-run-as.test.ts`, new `[#24497]` block (layer 1, a real
`ScopedContext` over a recording engine that answers the resolver's
reads). It covers:
- the hand-built shape carries the organisation's peers and nothing
beyond;
- carried positions and permission sets stay verbatim and only the peer
list is taken;
- a context with a list resolves nothing;
- no organisation gives the user alone;
- `'system'` and `'inherit'` resolve nothing, and the user-less refusal
holds;
- a resolver failure keeps the unseeded pin and warns once.
- **Moved pin, one:** `'user' DE-ELEVATES` read `engine.seen[0]`, which
is now the resolver's first read. It now reads the hook's own `update`
and still asserts `isSystem: false` and `userId`. Swept for kin:
`runtime/src/sandbox/hook-run-as.integration.test.ts` pins only the
user-less refusal for `'user'`, which does not move. Every other `runAs`
test outside these files pins flows or `'system'`.
- The real-stack probe above (5 cases).
## Ablation (committed first, mutated through
`scripts/ablation-replace.mjs`)
- **Mutation:** the seed (`withUserGrantsSeeded` at `e82725a18`;
`[SEED_USER_GRANTS]` from `f094a9d49`, re-ablated in fix round 1)
returns `this` unconditionally. The marker was proved in `dist/` by
`ablation-dist-preflight.mjs`, in 4 built files.
- **Integration:** 2 red, the rest green:
- `expected [ 'usr_me' ] to deeply equal [ 'usr_me', 'usr_peer' ]` (the
card's shape);
- `expected undefined to deeply equal [ 'usr_me' ]` (approvals'
envelope).
- The control and no-organisation cases stay green.
- **Unit:** 4 red: the hand-built, verbatim, no-organisation and
resolver-failure pins.
- **Restore:** blob equal to HEAD, `git diff HEAD` empty, rebuilt,
marker `--absent` from all 14 built files. 5/5 and 20/20 green again.
## Tests and gates (head `e82725a18`, which merges `origin/main` at
`3ae0b777d`)
- `pnpm --filter @objectstack/objectql typecheck`: exit 0
(`check:test-typecheck: OK`, debt held).
- `pnpm --filter @objectstack/service-automation typecheck`: exit 0.
- Both packages rebuilt after the merge.
- objectql: the 49 test files that reach `ScopedContext`, the hook
wrappers, `runAs` or `ctx.api`: 49 passed, 860 tests.
- The full objectql suite does not fit the container's 10-minute
foreground cap (killed at 580 s). This narrowing is declared; `Test
Core` runs the full suite.
- service-automation `runas-*` and `crud-runas`: 6 files, 55 tests
passed.
- `pnpm --filter @objectstack/core test:repo`: 6 files, 82 tests passed.
`pnpm --filter @objectstack/objectql test:repo`: 1 file, 5 tests passed.
- Census rows: none owed. The `[#22661]` census keys on
`referenceTargetOf` and `federated-injected-column-readers` keys on
injected-column seams; this diff calls neither. Both pass.
- **Derived gates** (`node scripts/pm/dispatch-gates.mjs --commands`, no
paths, 72 commands): all 72 exit 0.
- `check:system-context-census` first went red on a new elevation read
in the seed method. The read was removed (`e82725a18`), not added as a
census row.
- `check:dual-build-cjs-loads` first exited 3 (`PREREQUISITE NOT MET`).
After a full `turbo run build`, it exited 0.
- `--ran`: `72 derived famil(ies) accounted for — 72 run, 0
NOT-MEASURED`.
## Acceptance notes
- **The approvals writes still see only the acting user's own row.** The
status mirror (`mirrorStatusField`) deliberately carries no `tenantId`,
and `approval-node.ts` carries `organizationId`, which is not an
envelope field, rather than `tenantId`. Either way the seeded list is
the organisation-less one, the user alone, as the table shows.
- What moves for them today is that the list exists. Under #24341's
one-policy default (`id == current_user.id || id in
current_user.org_user_ids`), such a hook keeps its own row instead of
losing it.
- Seeing peers would need the producer to name the organisation it acts
in. That is a `plugin-approvals` producer change (`domain:services`),
not made here, and not by reading `organizationId` as an alias.
- **Positions and permission sets.** A hand-built context with no
`permissions` now takes the user's resolved grants. Today such a hook
falls back to the security plugin's member baseline, the #3356 gap flows
closed. The approvals' envelope carries `positions: []` and
`permissions: []`, which stay verbatim by the same rule that keeps a
narrowed ceiling, so those hooks keep the baseline. A separate gap, on
the producer.
- **Not carried into the hook's pin, as on the flow side:**
`systemPermissions`, `accessible_org_ids` and `posture`. Read-only, not
probed.
- **Public surface:** no nameable member is added to `ScopedContext`.
The seed is keyed on a module symbol (`SEED_USER_GRANTS`) that the
package entry does not export; the built `.d.ts` was checked (fix round
1, report 6266560805). No new index symbol and no spec key: `Clause-②:
no`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01PdsDLkA7E6UZywu8MVmeWa)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(objectql): a `runAs: 'user'` hook now sees the acting user's organization members in `sys_user`, as the same user's own request does
6
+
7
+
A hook declaring `runAs: 'user'` is pinned to the triggering user. When the triggering write carried a hand-built system context such as `{ isSystem: true, userId, tenantId }` (the approvals writes are the shipped producers), the pinned context carried no `org_user_ids`, the peer list the request path resolves for every user. The shipped `sys_user_org_members` policy (`id in current_user.org_user_ids`) therefore dropped out, and the hook's `ctx.api` read of `sys_user` returned the acting user's own row only, while the same user's request returned their organization's members too.
8
+
9
+
The hook wrapper now seeds the pinned context through the resolver the request path uses (`resolveUserAuthzGrants` from `@objectstack/core`):
10
+
11
+
- The peer list is the members of the context's organization (`tenantId`) plus the acting user. A context with no organization gets the acting user alone, which is what an organization-less request gets. A user who belongs only to other organizations is never in it.
12
+
- A context with no `permissions` also takes the user's resolved positions and permission sets, so the hook acts with the user's real grants rather than the security plugin's member baseline. Positions and permission sets the context already carries stay exactly as they were, so a narrowed ceiling is not re-broadened; only the peer list is added.
13
+
- A context that already carries a peer list, such as a request-resolved one, resolves nothing and is handed through as before.
14
+
- If the resolver fails, the hook runs on the unseeded pin, as before, and a warning says so. Policies that read `org_user_ids` then drop out: the hook sees less, never more.
15
+
16
+
`runAs: 'inherit'` (the default) and `runAs: 'system'` are unchanged, and a `runAs: 'user'` hook whose trigger names no user is still refused with `HOOK_UNSCOPED_DATA_ACCESS`. A `runAs: 'user'` hook fired from a context that lacks either list now pays one grants resolution per dispatch.
17
+
18
+
Clause-②: no (a runtime producer fix: a `runAs: 'user'` hook reads what the same user's own request reads. Nothing `@objectstack/spec` declares moves; owes the seat's review of record before the queue)
it('a hand-built { isSystem: true, userId, tenantId } trigger: the pin carries the organisation\'s peers, the user included, and nothing beyond',async()=>{
0 commit comments