Skip to content

Commit 4a37192

Browse files
committed
test(dogfood): migrate the flow-runAs write leg to the ruled 400 FLOW_FAILED
The `runAs:'user'` WRITE leg triggers a flow whose `update_record` node is refused at the record layer, so the run fails — and since the trigger route answers real HTTP status codes it answers 400 FLOW_FAILED instead of 200 wrapping an inner {success:false}. `memberTrigger`'s blanket `< 300` was written against the old contract and is the only dogfood consumer this change touches. The semantic the file pins is unchanged and still primary: the run executes AS the member, so the admin's note stays 'new'. What moves is the transport expectation, and it is asserted precisely rather than as a widened band — status 400, error.code FLOW_FAILED, the node-first access-refusal text, and the `touch` node's failure entry in error.details.summary. A 403 or 500 here would mean de-elevation broke differently and must not pass. The READ leg is deliberately NOT migrated: an RLS-scoped read is FILTERED, not refused, so that run still succeeds with an empty `found` and keeps its `< 300` expectation. Measured locally, not assumed. That leg also gains discrimination for free: until now a failed run and an empty read were indistinguishable there, because both left `found` falsy under HTTP 200. Verified locally on a built workspace closure: vitest run test/flow-runas.dogfood.test.ts → 5 passed (was 1 failed) flow-node · flow-function-effect · flow-durable-suspend · showcase-declarative-mcp → 17 passed showcase-anonymous-deny-surfaces · authz-conformance → 46 passed pnpm --filter @objectstack/dogfood typecheck → green Every other dogfood trigger caller expects the run to SUCCEED (or asserts an anonymous 401), so none is touched by this contract. No changeset change: @objectstack/dogfood is private internal QA and the wire change is already documented in .changeset/automation-trigger-status-unification.md.
1 parent 2116dde commit 4a37192

1 file changed

Lines changed: 70 additions & 2 deletions

File tree

packages/qa/dogfood/test/flow-runas.dogfood.test.ts

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@
1919
// object the member cannot read or write directly:
2020
// • system flows succeed on the admin's note → elevation is REAL,
2121
// • user flows are RLS-denied on the same note → de-elevation is REAL.
22+
// The two user-mode legs are denied DIFFERENTLY, and both shapes are asserted:
23+
// the WRITE is refused at the record layer, so the run fails and the trigger
24+
// route answers 400 `FLOW_FAILED` (#9378 — it rode HTTP 200 with an inner
25+
// `{success:false}` until then); the READ is filtered by RLS, so the run
26+
// SUCCEEDS with an empty `found`. Neither is a status band: a write leg that
27+
// started answering 403/500, or a read leg that started failing outright, is a
28+
// different bug and must not pass here.
2229
// Before the #1888 fix the user flows wrongly succeed (CRUD nodes passed no
2330
// identity → security skipped) → this file is RED; after the fix → GREEN.
2431

@@ -61,7 +68,18 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
6168
return (j.record ?? j).status;
6269
}
6370

64-
/** Trigger a flow as the restricted member; returns the inner AutomationResult. */
71+
/**
72+
* Trigger a flow as the restricted member and require the run to have
73+
* COMPLETED; returns the inner AutomationResult.
74+
*
75+
* [#9378] Since the trigger route answers real HTTP status codes, this helper
76+
* is also the discriminator it could not be before: a run that fails now
77+
* comes back 400 and is rejected HERE. Until then a failed run rode HTTP 200
78+
* with `{success:false}` inside, so the read leg below — which only checks
79+
* that `found` is falsy — passed identically whether the RLS-scoped read
80+
* returned EMPTY (the thing it means to prove) or the run DIED before
81+
* reading anything at all.
82+
*/
6583
async function memberTrigger(flow: string, noteId: string): Promise<{ success?: boolean; output?: any }> {
6684
const res = await stack.apiAs(memberToken, 'POST', `/automation/${flow}/trigger`, { params: { noteId } });
6785
expect(res.status, `trigger ${flow} HTTP failed: ${res.status} ${await res.clone().text()}`).toBeLessThan(300);
@@ -70,6 +88,50 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
7088
return body.data ?? {};
7189
}
7290

91+
/**
92+
* Trigger a flow as the restricted member and require the run to have RUN AND
93+
* FAILED on a record-access refusal — the de-elevation proof's write leg.
94+
*
95+
* [#9378] The transport contract this asserts, in full rather than by status
96+
* band: the route answers **400** `FLOW_FAILED` (ADR-0112 envelope), the node
97+
* failure is `error.message` verbatim, and the per-node accounting in
98+
* `error.details.summary` names WHICH node failed. Asserting the band
99+
* (`>= 400`) or the throw alone would keep passing if the refusal turned into
100+
* a 403 authorization verdict or a 500 fault — both of which would mean the
101+
* de-elevation broke in a different way, and both of which this file exists
102+
* to catch.
103+
*
104+
* Before #9378 this same run answered `HTTP 200 {"success":true,"data":{
105+
* "success":false,…}}`, so the ONLY visible trace of the denial was the
106+
* record staying unchanged. That assertion is still below and still the
107+
* primary proof; this one is the transport half that used to be invisible.
108+
*/
109+
async function memberTriggerExpectingAccessRefusal(flow: string, noteId: string, failingNodeId: string) {
110+
const res = await stack.apiAs(memberToken, 'POST', `/automation/${flow}/trigger`, { params: { noteId } });
111+
const text = await res.clone().text();
112+
expect(res.status, `trigger ${flow} should be 400 FLOW_FAILED: ${res.status} ${text}`).toBe(400);
113+
114+
const body = (await res.json()) as {
115+
success?: boolean;
116+
data?: unknown;
117+
error?: { code?: string; message?: string; httpStatus?: number; details?: { summary?: { nodes?: any[] } } };
118+
};
119+
expect(body.success).toBe(false);
120+
expect(body.error?.code, `expected FLOW_FAILED: ${text}`).toBe('FLOW_FAILED');
121+
expect(body.error?.httpStatus).toBe(400);
122+
// The double envelope is GONE, not re-labelled: nothing is left for a
123+
// status-blind caller to misread as a successful run.
124+
expect(body.data).toBeUndefined();
125+
// The failure is the RLS refusal on the note, named node-first — not a
126+
// generic "flow failed", which would pass while the run died of anything.
127+
expect(body.error?.message).toContain(`Node '${failingNodeId}' failed`);
128+
expect(body.error?.message).toMatch(/do not have access to this record/i);
129+
// Which node failed survives the envelope change — the reason `summary`
130+
// rides in `details` at all.
131+
const failed = body.error?.details?.summary?.nodes?.find((n) => n?.nodeId === failingNodeId);
132+
expect(failed?.status, `no failure entry for node '${failingNodeId}' in ${text}`).toBe('failure');
133+
}
134+
73135
it('precondition: the automation service is wired and a flow is registered', async () => {
74136
const res = await stack.apiAs(memberToken, 'GET', '/automation/runas_system_touch');
75137
expect(res.status, `automation service not wired: ${res.status}`).toBe(200);
@@ -102,7 +164,13 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
102164

103165
it("runAs:'user' DE-ELEVATES — member-triggered user flow is RLS-DENIED on the same record", async () => {
104166
const id = await adminCreateNote('user-touch');
105-
await memberTrigger('runas_user_touch', id);
167+
// The de-elevated run reaches the record layer as the MEMBER and the write
168+
// is refused there, so the run fails on its `touch` node — surfaced since
169+
// #9378 as 400 `FLOW_FAILED` instead of a 200 wrapping the inner failure.
170+
// The refusal text is asserted in the helper: this leg proves the identity
171+
// switch is real, so "denied because of WHO ran it" is the load-bearing
172+
// part, not merely "something went wrong".
173+
await memberTriggerExpectingAccessRefusal('runas_user_touch', id, 'touch');
106174
// The run executed as the member; the by-id write to the admin's note is
107175
// RLS-denied, so the record is unchanged. (Before the fix it would read
108176
// 'touched-user' — the privilege-boundary surprise this gate pins.)

0 commit comments

Comments
 (0)