Skip to content

Commit 14ccb1d

Browse files
fix(service-automation): a runAs:'user' flow carries its user's email, so an email-keyed row policy admits the run as it admits the user's own request (#24636)
Fixes #24613 Clause-②: yes (widening). `AutomationContext.email` and the exported `FlowUserGrants` / `RunIdentityContext` types gain one optional member each. A run reaches rows its user's own request reaches, and nothing beyond. ## What was wrong A `runAs: 'user'` flow's data context carried no `email`. The RLS compiler binds `current_user.email` from the execution context's `email` and nothing else, so every row policy keyed on it (the stock showcase's `task_own_rows`, `assignee == current_user.email`) dropped out inside the run. A record-change flow triggered by a member whose only access to a row is such a policy was refused on that row, while the member's own request reads and updates it. The request path gets the email from the core resolver (`resolveUserAuthzGrants`, through `resolveAuthzContext`); the run called the same resolver and dropped the field at every step. ## The change - **The bridge passes the resolver's email on** (`plugin.ts`). The grants resolver the plugin wires already called `resolveUserAuthzGrants`; it now returns `grants.email` beside the positions, permission sets and peer list. `FlowUserGrants` gains an optional `email`. - **Run setup seeds it** (`seedRunGrants`, `engine.ts`), by the rule the peer list uses: only what the context lacks. A `runAs: 'user'` context with a user and no email is resolved and takes the email; a context that already carries one keeps it verbatim (the request path keeps a session's email the same way). Forwarded positions and permission sets stay untouched. - **The decider's segment carries the decider's email** (`resolveDecisionSegmentContext`). It now removes the submitter's `email` with the submitter's other facts, on both postures, and `seedRunGrants` resolves the decider's. Without the removal the decider would run with the submitter's email under the decider's id. - **`resolveRunDataContext` stays the single mapping** (`runtime-identity.ts`). Its `runAs: 'user'` arm copies the email onto the data context (`RunIdentityContext.email`, optional). No email is invented when none was seeded. The `runAs: 'system'` arm is untouched. - **Contract** (`packages/spec/src/contracts/automation-service.ts`, a spec-lane rider): `AutomationContext.email?: string` with TSDoc naming who sets it, who reads it, and what its absence means. It regenerates no artefact: `pnpm --filter @objectstack/spec check:generated` reports all 14 generated artefacts up to date. - The `[runAs]` warning on a resolver failure names the email beside the peer list. - Docblocks that said no run context carries an email are corrected (`runUserOf`, the CEL scope note). ## Measurement: the card's reproduction on the real stack `packages/services/service-automation/src/runas-user-email-owner.integration.test.ts`, beside `runas-user-hook-org-peers.integration.test.ts`. It boots ObjectKernel, ObjectQLPlugin, driver-sql on better-sqlite3 `:memory:`, the identity objects, the real SecurityPlugin (shipped sets plus one fixture set: `select` and `update` on `assignee == current_user.email`) and the real AutomationEngine with its CRUD nodes. Runs start with the record-change trigger's own envelope (`record`, `object`, `event`, `userId`, `tenantId`, `params`). `@objectstack/trigger-record-change` depends on this package, so it is not composed here. Before: the three producer files at `c3954057f` (`git restore --source`, each file's blob hash checked against the base blob), the new pin at head. After: head. | case | before | after | |:--|:--|:--| | CONTROL: the member's own request reads and updates the row | pass | pass | | GUARD: the same request without the email does not see the row | pass | pass | | the card's reproduction (record-change run, read and stamp) | **fail**: `update_record(qa_owner_task) failed: Record ID not found in qa_owner_task` | pass, `notes` equals the member's id | | the doors' shape (positions and permission sets forwarded, no email) | **fail**, same message | pass | | nothing beyond: a row assigned to someone else's email | pass (refused) | pass (refused) | | after a decision, the decider's row is stamped with the decider | **fail**, same message | pass | | after a decision, the submitter's row is refused to the decider | pass (refused) | pass (refused) | | CONTROL: a `runAs: 'system'` run | pass | pass | Before-leg totals: `Tests 3 failed | 5 passed (8)`. After: `Tests 8 passed (8)`. The card's showcase run reported the record-level 403 sentence; this stack answers the not-found that by-id writes of an unreadable row answer. Both are the refusal of a row the member can read. ## Ablations (committed first, mutated through `scripts/ablation-replace.mjs`, each restored to the HEAD blob with `git diff HEAD` empty) The subjects resolve through relative imports to `src/`, so no build sits between the mutation and the run. 1. **Drop the seed** (`if (needsEmail && grants.email) runContext.email = grants.email;` deleted): 7 red across three files. The card's reproduction, the doors' shape and the decider's row on the real stack (`Record ID not found`), the decider envelope (`after.email`), and three unit pins in `crud-runas.test.ts`. `Tests 7 failed | 59 passed (66)`. 2. **Keep the submitter's email in the decider's segment** (`delete switched.email;` deleted): 6 red. On the real stack the submitter's row is written by the decider (`expected true to be false`) and the decider's own row is refused. In the envelope layer: `expected 'rep@example.test' to be 'mgr@example.test'`, the failed-resolution case (`expected 'rep@example.test' to be undefined`), and both `runAs: 'system'` decision cases. `Tests 6 failed | 23 passed (29)`. Reverse type check: an `AutomationContext` literal with `email: 42` planted in `runtime-identity.ts` gives `TS2322: Type 'number' is not assignable to type 'string'` under `tsc --noEmit`. So the type check reads the rebuilt `.d.ts` that declares the member. A stale one would report an unknown property. Restored to the HEAD blob. ## Tests and gates (head `5318e5810`, which merges `origin/main` at `973917c57`) - `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2`: `Test Files 193 passed (193)`, `Tests 2580 passed (2580)`. - `pnpm --filter @objectstack/service-automation typecheck`: exit 0 (`tsc --noEmit`, then `check:test-typecheck: OK`). - `pnpm --filter @objectstack/spec typecheck`: exit 0. `vitest run --project local --maxWorkers=2` (the `test` script's runner): `Test Files 701 passed (701)`, `Tests 21312 passed | 1 todo`. Spec run at `0663fe26c`; the merge moved no file under `packages/spec`. - `pnpm --filter @objectstack/spec check:generated`: all 14 generated artefacts up to date. - Derived gates (`node scripts/pm/dispatch-gates.mjs --commands`, 90 commands) on `5318e5810`: all 90 exit 0, each recorded with its exit code. `--ran`: `90 derived famil(ies) accounted for — 90 run, 0 NOT-MEASURED` (0 UNRUN). An earlier run at `0663fe26c` had one exit 3, `check:dual-build-cjs-loads` (`PREREQUISITE NOT MET`, it reads every package's `dist/`). It exited 0 once the dists were built, and on the final head it exits 0 on the first run. - `check:dev-prereqs` (local-only family; this diff touches `packages/spec`): exit 0. - `test:repo` is not run locally: the diff edits no repo-project test, and `Test Core` runs it. ## Acceptance notes - **The hook-side twin is not in this PR.** `ScopedContext[SEED_USER_GRANTS]` (`packages/objectql`) seeds a `runAs: 'user'` hook's positions, permission sets and peer list, but not the email. It is outside this card's surface and is reported to the seat for the `domain:engine` lane. A hook fired by a `runAs: 'user'` flow's write now inherits the run's email through the write's context. - **The flow CEL `current_user` still binds no email.** The run context now holds one for a `runAs: 'user'` run. The binding follows the ruled list (id, positions, organization, the platform-admin flag), and the email serves the data operations' row policies. A pin in `flow-cel-current-user.test.ts` holds that. Whether the binding should widen is a question for the seat, not a change made here. - **Persistence.** A paused `runAs: 'user'` run's `sys_automation_run.context_json` now carries the user's email beside the positions and permission sets it already carried. That row's read is governed by `sys_automation_run`'s own permissions. - **Runs suspended before this release** resume without the email (a resume does not re-run setup) and keep today's answer until they end. - **Cost.** A context that carries permissions and a peer list but no email now asks the resolver once, for the email alone. That shape is a `subflow` / `map` child of an email-less user, or an in-process caller that resolved the rest itself. Every door-started run already paid this resolution for the peer list. - **Contract pin.** No type-level pin file was added under `packages/spec/src/contracts/`: the claim's surface names only `automation-service.ts` there. The member's type is exercised by the service-automation test layer (`check:test-typecheck`) and by the reverse type check above. --- _Generated by [Claude Code](https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 35450f0 commit 14ccb1d

9 files changed

Lines changed: 527 additions & 41 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/service-automation': minor
3+
'@objectstack/spec': minor
4+
---
5+
6+
fix(service-automation): a `runAs: 'user'` flow now carries its user's email, so a row policy on `current_user.email` admits the run on the rows it admits for the same user's own request
7+
8+
Clause-②: yes (widening)
9+
10+
A `runAs: 'user'` flow's data nodes ran without the acting user's email, which the request path resolves for every user. The row-level security compiler reads `current_user.email` from the execution context's `email` and nothing else, so every owner policy keyed on it (such as `assignee == current_user.email`) dropped out inside the run. A record-change flow triggered by a member whose only access to a row is such a policy was refused on the row the member had just written, and could read and update with their own request.
11+
12+
Run setup now seeds the email through the resolver the request path uses (`resolveUserAuthzGrants` from `@objectstack/core`, the user's stored `sys_user` email), and every data node passes it on:
13+
14+
- It is seeded for every trigger shape that names a user and carries no email: the record-change shape (`userId` and `tenantId` only) and the REST and action doors' shape (positions and permission sets already resolved). Only the missing facts are taken from the resolver; forwarded positions and permission sets stay exactly as they were.
15+
- A context that already carries an email keeps it verbatim, as a request's session email wins over the stored one. A `subflow` or `map` child and a suspended run keep the email their run was seeded with.
16+
- After an approval decision, the nodes that run as the decider carry the decider's own email, resolved for them. The submitter's email is removed with the rest of the submitter's grants, on both `runAs` postures.
17+
- A user with no stored email, or a resolver failure, leaves the run without one: policies that read it drop out, and the run sees less, never more. The `[runAs]` warning on a resolver failure now names the email beside the peer list.
18+
- A run suspended before this release resumes without the email until it ends.
19+
20+
A `runAs: 'system'` flow is unchanged. The flow CEL scope's `current_user` is unchanged too: it still binds no email.
21+
22+
Published types: `AutomationContext` (`@objectstack/spec/contracts`) gains an optional `email` member, and `RunIdentityContext` / `RunDataContext` and `FlowUserGrants` (`@objectstack/service-automation`) each gain an optional `email` member.

‎packages/services/service-automation/src/builtin/crud-runas.test.ts‎

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ describe("runAs:'user' resolves the triggering user's grants at run setup (#3356
449449
const seen: Array<{ userId: string; tenantId?: string }> = [];
450450
engine.setUserGrantsResolver((userId, tenantId) => {
451451
seen.push({ userId, tenantId });
452-
return { positions: ['SHOULD_NOT_APPLY'], permissions: ['SHOULD_NOT_APPLY'], org_user_ids: ['u1', 'u2'] };
452+
return { positions: ['SHOULD_NOT_APPLY'], permissions: ['SHOULD_NOT_APPLY'], org_user_ids: ['u1', 'u2'], email: 'u1@example.test' };
453453
});
454454

455455
// An already-resolved envelope — e.g. an ADR-0090 agent ceiling acting
@@ -463,10 +463,11 @@ describe("runAs:'user' resolves the triggering user's grants at run setup (#3356
463463
expect(c.ctx.permissions).toEqual(['mcp_agent_read']);
464464
expect(c.ctx.positions).toEqual(['agent']);
465465
expect(c.ctx.org_user_ids, `${c.op} lost the seeded peer list`).toEqual(['u1', 'u2']);
466+
expect(c.ctx.email, `${c.op} lost the seeded email`).toBe('u1@example.test');
466467
}
467468
});
468469

469-
it('resolves nothing when the trigger already carried permissions AND a peer list (a subflow child, a resumed leg)', async () => {
470+
it('resolves nothing when the trigger already carried permissions, a peer list AND an email (a subflow child, a resumed leg)', async () => {
470471
const engine = new AutomationEngine(makeLogger());
471472
const { data, calls } = fakeData();
472473
registerCrudNodes(engine, ctxWith(data));
@@ -475,20 +476,93 @@ describe("runAs:'user' resolves the triggering user's grants at run setup (#3356
475476
let called = 0;
476477
engine.setUserGrantsResolver(() => {
477478
called++;
478-
return { positions: ['SHOULD_NOT_APPLY'], permissions: ['SHOULD_NOT_APPLY'], org_user_ids: ['SHOULD_NOT_APPLY'] };
479+
return { positions: ['SHOULD_NOT_APPLY'], permissions: ['SHOULD_NOT_APPLY'], org_user_ids: ['SHOULD_NOT_APPLY'], email: 'SHOULD_NOT_APPLY' };
479480
});
480481

481482
await engine.execute('usr', {
482-
userId: 'u1', positions: ['member'], permissions: ['member_default'], tenantId: 't1', org_user_ids: ['u1', 'u3'],
483+
userId: 'u1', positions: ['member'], permissions: ['member_default'], tenantId: 't1', org_user_ids: ['u1', 'u3'], email: 'u1@example.test',
483484
} as SeededRunContext);
484485
expect(called).toBe(0);
485486
expect(calls.length).toBeGreaterThan(0);
486487
for (const c of calls) {
487488
expect(c.ctx.permissions).toEqual(['member_default']);
488489
expect(c.ctx.org_user_ids).toEqual(['u1', 'u3']);
490+
expect(c.ctx.email).toBe('u1@example.test');
491+
}
492+
});
493+
494+
it('a context carrying permissions and a peer list but no email resolves the email alone, and keeps the rest verbatim', async () => {
495+
const engine = new AutomationEngine(makeLogger());
496+
const { data, calls } = fakeData();
497+
registerCrudNodes(engine, ctxWith(data));
498+
engine.registerFlow('usr', allOpsFlow('usr', 'user'));
499+
500+
const seen: Array<{ userId: string; tenantId?: string }> = [];
501+
engine.setUserGrantsResolver((userId, tenantId) => {
502+
seen.push({ userId, tenantId });
503+
return { positions: ['SHOULD_NOT_APPLY'], permissions: ['SHOULD_NOT_APPLY'], org_user_ids: ['SHOULD_NOT_APPLY'], email: 'u1@example.test' };
504+
});
505+
506+
await engine.execute('usr', {
507+
userId: 'u1', positions: ['member'], permissions: ['member_default'], tenantId: 't1', org_user_ids: ['u1', 'u3'],
508+
} as SeededRunContext);
509+
expect(seen).toEqual([{ userId: 'u1', tenantId: 't1' }]);
510+
expect(calls.length).toBeGreaterThan(0);
511+
for (const c of calls) {
512+
expect(c.ctx.positions).toEqual(['member']);
513+
expect(c.ctx.permissions).toEqual(['member_default']);
514+
expect(c.ctx.org_user_ids).toEqual(['u1', 'u3']);
515+
expect(c.ctx.email, `${c.op} lost the seeded email`).toBe('u1@example.test');
516+
}
517+
});
518+
519+
it('seeds the email beside the resolved grants and threads it to EVERY data op', async () => {
520+
const engine = new AutomationEngine(makeLogger());
521+
const { data, calls } = fakeData();
522+
registerCrudNodes(engine, ctxWith(data));
523+
engine.registerFlow('usr', allOpsFlow('usr', 'user'));
524+
engine.setUserGrantsResolver(() => ({ positions: ['org_member'], permissions: ['member_default'], org_user_ids: ['u1'], email: 'u1@example.test' }));
525+
526+
// The record-change trigger's shape: a user id and an organization, no email.
527+
const res = await engine.execute('usr', { userId: 'u1', tenantId: 't1' });
528+
expect(res.success).toBe(true);
529+
expect(calls.length).toBeGreaterThan(0);
530+
for (const c of calls) expect(c.ctx.email, `${c.op} lost the seeded email`).toBe('u1@example.test');
531+
});
532+
533+
it('a context that already carries an email keeps it verbatim; the resolver\'s stored one is not applied', async () => {
534+
const engine = new AutomationEngine(makeLogger());
535+
const { data, calls } = fakeData();
536+
registerCrudNodes(engine, ctxWith(data));
537+
engine.registerFlow('usr', allOpsFlow('usr', 'user'));
538+
let called = 0;
539+
engine.setUserGrantsResolver(() => {
540+
called++;
541+
return { positions: ['org_member'], permissions: ['member_default'], org_user_ids: ['u1'], email: 'SHOULD_NOT_APPLY' };
542+
});
543+
544+
// A door that forwards the request's own (session) email.
545+
await engine.execute('usr', { userId: 'u1', tenantId: 't1', email: 'u1.session@example.test' });
546+
expect(called, 'the other facts are still resolved').toBe(1);
547+
expect(calls.length).toBeGreaterThan(0);
548+
for (const c of calls) {
549+
expect(c.ctx.email).toBe('u1.session@example.test');
550+
expect(c.ctx.permissions).toEqual(['member_default']);
489551
}
490552
});
491553

554+
it('a resolver that answers no email seeds none — the data context carries no email, never a made-up one', async () => {
555+
const engine = new AutomationEngine(makeLogger());
556+
const { data, calls } = fakeData();
557+
registerCrudNodes(engine, ctxWith(data));
558+
engine.registerFlow('usr', allOpsFlow('usr', 'user'));
559+
engine.setUserGrantsResolver(() => ({ positions: ['org_member'], permissions: ['member_default'], org_user_ids: ['u1'] }));
560+
561+
await engine.execute('usr', { userId: 'u1', tenantId: 't1' });
562+
expect(calls.length).toBeGreaterThan(0);
563+
for (const c of calls) expect('email' in c.ctx, `${c.op} carried an invented email`).toBe(false);
564+
});
565+
492566
it('seeds the peer list beside the resolved grants and threads it to EVERY data op', async () => {
493567
const engine = new AutomationEngine(makeLogger());
494568
const { data, calls } = fakeData();
@@ -529,6 +603,7 @@ describe("runAs:'user' resolves the triggering user's grants at run setup (#3356
529603
expect(c.ctx.isSystem).toBe(false);
530604
expect(c.ctx.permissions).toEqual(['member_default']);
531605
expect('org_user_ids' in c.ctx).toBe(false);
606+
expect('email' in c.ctx).toBe(false);
532607
}
533608
});
534609

0 commit comments

Comments
 (0)