Skip to content

Commit f40bb32

Browse files
fix(service-automation): flow write nodes refuse a stored-metadata family target (#21624) (#21649)
Part of #21624 Clause-②: no This PR carries the card's run-time half. The save-time half is not in this package; it is named below for the seat as an open question, and #21624 remains open for it. ## What this changes A flow's `create_record`, `update_record` and `delete_record` nodes no longer write the stored-metadata family (the current metadata table and its version history, judged by the family's own predicate `isStoredMetadataBodyObject` from `@objectstack/spec/kernel`). Triage's ruling on the card, "refuse at the node", applies the reason of #21520's ruling A (the family has one writer for app-authored work, the metadata protocol) to flows. - `crud-nodes.ts` gains `storedMetadataWriteRefusal(nodeType, objectName)`. For a family target it returns a guard refusal (`refuseNode`, the node's existing channel) that carries the standard catalog's `PERMISSION_DENIED`. The message names the metadata API and the metadata protocol as the way to change metadata, and says that elevation does not change this. Any other object gets `undefined`. - Each of the three write executors calls it right after the config parse and the `objectName` check. That is before the filter is interpolated or the erased-condition guard runs, before `fields` is resolved, and before the data engine is looked up. So a refused node answers the same whatever its filter or payload names, and nothing is computed or written. This also shuts the write nodes' evaluate exit: their `filter` is never run against a family table. - The name judged is the exact `objectName` string the executor hands the engine. The write executors do not interpolate `objectName`, which was measured: a `{token}` target reaches the engine as the literal template. - ⛔ The node does not route the write through the protocol. That would be a second write path into the family. Nothing is copied from another package, and there is no new dependency edge. - **Rider (declared in the claim):** the `NodeExecutionResult.code` docblock in `engine.ts` named one executor as "the only executor that sets it today". `get_record` (PR #21641) and now the three write nodes set it too. The sentence now names each executor and the code it sets. Files: `crud-nodes.ts`, `engine.ts` (the docblock rider, one sentence), a new pin file and a changeset. There is no `metadata-protocol`, `packages/spec` or lint edit. ## Census, taken before any edit The census asked which shipped flow has a `create_record`, `update_record` or `delete_record` node whose target is a family table. A read-only scanner found every write-node literal (`type: 'WRITE_NODE'` in TS, JS, JSON, YAML and MD) and read the `objectName` / `object` key from the same node object. A literal target was classified as family or non-family. A non-literal target (a variable, a template, or none) was listed and read by hand. | Root (tree) | Write nodes found | Family targets | Non-literal targets | |---|---|---|---| | objectstack `packages/**`, non-test (`96b0e3108a`) | 24 | 0 | 7, all of them the executors' own descriptor forms and one README fragment, none a flow | | objectstack `examples/**`, non-test | 9 (9 of the 9 write-node sites `git grep` finds) | 0 | 0 | | objectstack `skills/**` | 3 | 0 | 0 | | objectstack `content/docs/**` | 15 | 0 | 10 (doc fragments) | | hotcrm, shallow clone at `9466837` | 38 | 0 | 0 | | test files (objectstack packages and examples; hotcrm) | 182; 9 | 0; 0 | 39; 5, read by hand, none a family name | - **Platform flows:** no platform package ships a flow with any write node. The 24 package hits are node descriptors, conversion-layer fixtures, the CLI's explain text and changelog examples. The production `registerFlow` callers are the flow loader and the `/automation` write doors; neither builds a flow of its own. - **Interpolated targets:** the executors read `objectName` raw and never interpolate it, which the reproduction below confirms. So a write node's target is always the literal in its config. The census's non-literal column covers TS-built configs. - **Positive controls:** a synthetic fixture with one `delete_record` and one `update_record` aimed at the two family tables gave 2 of 2 family hits. The same scanner, run over `get_record` in test files, listed the sibling read pins' parameterized family targets as non-literal (8 hits). The family names themselves occur in 383 non-test package files, for example the platform's own list views in `metadata-core`. - **Not measured:** cloud is not in the census triage named. Tenant flows stored in deployments are deployment data and are not in any checkout. Census result: zero. No shipped flow and no platform flow writes the family through a data node, so the refusal lands over no platform writer. ## Reproduction on `main` before the fix (by class) The composition was `ObjectKernel`, `ObjectQLPlugin`, the real `AutomationServicePlugin`, and `driver-sql` on better-sqlite3 `:memory:`. Every case was run without the security plugin and with the real `SecurityPlugin` (the default permission sets). No stored value is recorded here. - **Without the security plugin:** all three nodes, on both family tables, under both `runAs: 'system'` and `runAs: 'user'`, changed the table (12 of 12). That includes `delete_record`, which was not measured on the card. - **With the security plugin:** - `runAs: 'system'` changed the table in 6 of 6 cases (three nodes, two tables), since the elevated write skips the middleware. - `runAs: 'user'` was refused in 12 of 12 cases, for both a rank-and-file member and a platform administrator. The refusal came from the middleware's ADR-0103 engine-owned write guard. It surfaced as a routable runtime failure with no code, and the table was unchanged. - So in a secured composition the reach is the system identity, and it does not depend on grants. - **The write nodes' evaluate exit:** an `update_record` (`multi: true`, system identity) whose filter read the stored body column acted on the row (`acted: 1`) for a matching guess and on nothing (`acted: 0`) for a non-matching one. - **A target supplied by a variable** (`objectName: '{record.target}'`) is not interpolated: the engine was asked for the literal template, answered "not found", and the family table was unchanged. - **Spelling variants** (upper case, mixed case, leading or trailing space): 4 variants × 3 nodes × 2 compositions all answered "not found". The registry resolves exact names, so the exact-name predicate leaves no variant reach. ## The code: why `PERMISSION_DENIED` The dispatch asked which code the generic data door answers for a direct family write, so it could be reused. Two answers were measured: - **REST `/data/:object` write verbs on either table answer 405 `OBJECT_API_METHOD_NOT_ALLOWED`.** That answer comes from `apiAccessDenialFromEnable` over the tables' `apiMethods: ['get', 'list']`. Measured through `apiExposureDenialReason`, the spec helper that function wraps: create, update and delete give `method-not-allowed`. - **The door's in-process write path** (`createData` / `updateData` / `deleteData`) **answers a non-platform principal with `PERMISSION_DENIED` / 403** in a secured composition, for both member and administrator. The system context is admitted, since it is the platform's own path. The node carries `PERMISSION_DENIED`. The 405 is the REST exposure gate, and its own docblock scopes it to the external API: "Internal callers (hooks, flows, raw objectql) are unaffected". The 403 is the door's answer to the condition the node refuses: a principal that is not the platform may not write these tables. It is also the code that ruling A's body-write boundary (`stored-metadata-body-boundary.ts`) carries for the same rule. No code is minted, and `PERMISSION_DENIED` is a standard catalog member. ## The save-time half is not in this package (open question for the seat) Triage's third pin asks that a flow naming a family target statically be refused at save. Read from the code, the save doors judge a flow as follows: - **The `/meta` save door** (Studio, REST `/meta`, MCP authoring) runs `saveMetaItem`, which does three things. It canonicalizes the flow through the automation engine; a throw there is caught and the raw body is kept. It then runs the per-type `FlowSchema` safeParse. Finally, the runtime authoring gate in `metadata-protocol` runs the `@objectstack/lint/runtime` rules. - **`registerFlow` is not on that path.** The engine registers the stored row afterwards, on the publish rebind. - **The judge every door shares is `FlowSchema.parse` in `packages/spec`.** `flow-node-config-refusals.ts` describes itself as "the one judge `FlowSchema.parse`, `AutomationEngine.registerFlow` (which parses first) and `objectstack validate` share". - **Precedent:** the same ruling's hook half put its save-time refusal in the spec's `HookSchema` (`data/hook.zod.ts`, `refuseBodyOnStoredMetadataTarget`). So the save-time refusal belongs outside `service-automation`, and per the claim nothing there is edited here. The position and the edit it needs are in the report on the card, as an open question for a claim revision. The run-time refusal alone shuts the reach. ## Pins New file: `packages/services/service-automation/src/builtin/write-nodes-stored-metadata-family-refusal.integration.test.ts`, 17 cases on the composition above. - **Refused, without the security plugin:** 6 cases (each node × each identity), each over both family tables. In every case: - the run fails, and the node downstream does not run; - the engine's `insert` / `update` / `delete` is never called on a family table, and the table snapshot is unchanged; - the refusal names the metadata protocol; - a flow reads `PERMISSION_DENIED` on `{$error.code}` and on a `try_catch` region's error variable. - **Refused, with the security plugin:** 3 cases, each node over both tables under `runAs: 'system'`, under `runAs: 'user'` as a member, and under `runAs: 'user'` as a platform administrator. The same assertions apply. - **Control:** the data door answers a member's and an administrator's create, update and delete on the family with `PERMISSION_DENIED` / 403. - **The evaluate exit (2 cases):** an `update_record` and a `delete_record` whose filter reads the stored body, with a matching and a non-matching guess, give the same refusal text, no engine write and an unchanged table. - **Guard routing:** with a `fault` edge on the node, the run still fails and the handler never runs, for all three nodes. - **A target supplied by a variable (2 cases, both identities):** the family table is unchanged and the engine's write verb is never called on it. - **Non-family control (2 cases, both identities):** an ordinary object is created, updated and deleted exactly as before. ## Ablation The fix was committed first. The direction was predicted before the run. With the refusal's effect removed, the 12 refused-case pins would go red, because the write runs and succeeds. The 5 controls would stay green: the data door control, the two variable-target cases (the refusal never applied to them) and the two non-family cases. - **Mutation:** `scripts/ablation-replace.mjs` replaced the family test at the head of `storedMetadataWriteRefusal` with an always-`undefined` return carrying a marker. The anchor went from 1 to 0 and the marker from 0 to 1, and the file's blob changed. The subject is reached through a relative `src` import, so no `dist` rebuild or preflight applies. - **Result:** 12 failed and 5 passed (17), matching the prediction. On every red case, the first assertion to fail was the run's success flag: the write ran and the run succeeded. - **Restore:** the tool reported the blob after restore equal to the HEAD blob and `git diff HEAD` empty. A bash trap (`git checkout HEAD --` on the absolute path, then a blob comparison) re-confirmed it with 0 diff lines. `git status --porcelain` showed 0 lines and the marker grep 0. The re-run gave 17 passed (17). - The ablation was run at `1e7d538944`, at `233d3206e7` and at `afbc55b764` (the final head), with identical readings each time. ## Verification Every reading is at HEAD `afbc55b764`. That head merged `origin/main` at `15fe567c9c` (three commits, none in this package) and then rebuilt the tree (`turbo run build` excluding docs, 72 of 72 tasks). Every heavy run went through `scripts/pm/os-verify-lock.sh`. - `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2` (the whole suite, the new pins included): Test Files 169 passed (169), Tests 2095 passed (2095), VERDICT command-exit 0. - `pnpm --filter @objectstack/service-automation typecheck`: VERDICT command-exit 0, and `check:test-typecheck` is OK. `tsc --listFiles` shows the new pin file in both `tsconfig.json` and `tsconfig.test.json`. - **Gates:** `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (no paths) derives 65 commands at `afbc55b764` (31 pnpm and 34 direct node), with no stale-tree warning. - All 65 were run there, and each exited 0. `--ran` reconciles them: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN (exit 0). - `check:dual-build-cjs-loads` measured 106 entries across 66 packages on the built tree. At the first pass (`1e7d538944`, unbuilt tree) it answered PREREQUISITE NOT MET (exit 3), which measured nothing. - `check:query-options-erasure` was red at that first pass: the pin file's snapshot read added one untyped engine-options site to the test surface (236 to 237). The read was typed in `233d3206e7`, and the gate holds at 236. - That first derivation was 2 commits behind `origin/main` (a workflow file and the SDUI manifest record changed). After the merge, the same 65 commands derive. - Outside the derived set, and so NOT MEASURED locally, belonging to CI: six workflow-valued families (the three shard attestations, the issue-citation census and the two test-completeness checks), the five CI-shell jobs these paths schedule (Test Core, Temporal Conformance, the Dogfood Regression Gate, Dogfood Verify CLI and Build Core) and the type-check lanes. - `check:nul-bytes` exited 0. No turbo-driven gate left an `AGENTS.md` block, and `git status --porcelain` was empty after each pass. - **Lint, as a proven narrowing (`pnpm lint` itself is CI's):** - Population, read from eslint's own config: 3 of the 4 changed paths are linted (`crud-nodes.ts`, `engine.ts` and the pin file). For the changeset, eslint answers "no matching configuration". - Count, from `--format json` with `--no-inline-config`: 0 errors and 0 warnings on the 3 linted files. - Invariance: `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`, no typed rules), so this diff cannot move the verdict on an untouched file. - A control-byte scan of the 4 changed files found none. ## Docs `content/docs/**` (outside `releases/`) and `skills/**` were searched for flow data nodes writing system or metadata tables. No sentence states that a write node may target them, so none is made false and nothing is edited. Notes: - The guard callout in `content/docs/automation/flows.mdx` has an "In practice that is …" list. It does not name this refusal (nor the read-node one), so the list is incomplete, not false. - The `{$error.code}` row there says "e.g. `create_record`'s `DUPLICATE_RECORD`", which stays true. ## Acceptance notes - **The prescription sentence now has a third copy.** The runtime's body boundary and the spec's hook refusal each hold it in a module-private constant, and this node words it for a flow. No shared constant exists that this package can import. Carrier: none. - **An unreleased changeset sentence is now incomplete (not edited).** `.changeset/21623-flow-read-node-evaluate-refusal.md` says "The write nodes are unchanged", which was true of that change. Both changesets compile into the same release, and this PR's changeset states this change. This PR does not edit a changeset it did not add. Carrier: none. - **In a secured composition, before this change, the middleware's refusal of a user-identity family write was routable and carried no code.** It is now the node's guard refusal with a code. That was measured on the base and is not filed separately, since this change supersedes it. - **`registerFlow` was deliberately not given a second static check.** It parses `FlowSchema` first, so the save-time judge, wherever the seat places it, reaches it without a copy here. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 759dbe9 commit f40bb32

4 files changed

Lines changed: 465 additions & 4 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation): a flow's `create_record`, `update_record` and `delete_record` nodes refuse a stored-metadata table as their target (#21624)
6+
7+
Clause-②: no
8+
9+
The two stored-metadata tables (the current metadata bodies and their version history) have one writer for app-authored work: the metadata protocol, where a change is validated and its provenance is recorded. A flow's write nodes wrote those tables directly, outside it. Under `runAs: 'system'` the write ran elevated, so the security middleware never judged it; under `runAs: 'user'` only a composition with the security plugin refused it, as a routable runtime failure with no code. A write node's `filter` was also evaluated against the stored rows, so whether the write acted answered a predicate over the stored body.
10+
11+
**What changes.** A `create_record`, `update_record` or `delete_record` node whose `objectName` is either table is refused before it resolves its filter or its field values and before any engine write, under either run identity. The refusal names the metadata API as the way to change metadata and carries the standard `PERMISSION_DENIED` code, the code the data door answers a non-platform principal's write to these tables with. It is a guard failure: the run fails, nothing downstream of the node runs, and a `fault` edge does not route it. A `try_catch` catch region reads the code on `{$error.code}`. Metadata is changed through the metadata API (`PUT /api/v1/meta/:type/:name`), never through a flow's data nodes.
12+
13+
**What does not change.** Every other object is created, updated and deleted exactly as before. `get_record` keeps serving these tables projected and keyed.

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,55 @@ function storedMetadataFilterRefusal(
323323
return undefined;
324324
}
325325

326+
/** [#21624] What each write node would have done, in its refusal's own words. */
327+
const STORED_METADATA_WRITE_VERB = {
328+
create_record: 'create a record in',
329+
update_record: 'update',
330+
delete_record: 'delete from',
331+
} as const;
332+
333+
/**
334+
* [#21624] Refuse a WRITE node aimed at the stored-metadata family
335+
* (`sys_metadata` / `sys_metadata_history`, judged by the family's own
336+
* predicate, {@link isStoredMetadataBodyObject}).
337+
*
338+
* The family has one writer for app-authored work: the metadata protocol,
339+
* where a change is validated and its provenance recorded (the ruling that
340+
* refuses a hook body bound to these tables, or a body's direct write to them,
341+
* applied to its own reason: a flow is app-authored automation too). Under
342+
* `runAs: 'system'` the engine writes elevated and cannot tell this write from
343+
* the platform's own internal writers, so the rule is applied here, at the
344+
* node. ⛔ Not routed through the protocol from inside the node: that would be
345+
* a second write path into the family.
346+
*
347+
* Judged on the object name the engine would be handed, before the node
348+
* resolves its `filter` or its `fields`, so a refused node answers the same
349+
* whatever it names: its filter is never evaluated against a family table
350+
* (the write nodes' evaluate exit) and nothing it would write is computed.
351+
*
352+
* The answer is a guard refusal ({@link refuseNode}: the metadata is wrong, and
353+
* re-running it unchanged never succeeds) carrying the standard catalog's
354+
* `PERMISSION_DENIED`: the code the data door's in-process write path answers
355+
* a non-platform principal's write to these tables with in a secured
356+
* composition, and the code the body-write boundary for the same ruling
357+
* carries. No code is minted. `undefined` for any other object.
358+
*/
359+
function storedMetadataWriteRefusal(
360+
nodeType: keyof typeof STORED_METADATA_WRITE_VERB,
361+
objectName: string,
362+
): (ReturnType<typeof refuseNode> & { code: string }) | undefined {
363+
if (!isStoredMetadataBodyObject(objectName)) return undefined;
364+
return {
365+
...refuseNode(
366+
`${nodeType}: refusing to ${STORED_METADATA_WRITE_VERB[nodeType]} '${objectName}': it holds stored `
367+
+ 'metadata, and a flow may not write it directly, so the write was not run. Change metadata through the '
368+
+ 'metadata API (`PUT /api/v1/meta/:type/:name`, the metadata protocol), where it is validated and its '
369+
+ "provenance is recorded. Elevation (`runAs: 'system'`) does not change this.",
370+
),
371+
code: StandardErrorCode.enum.PERMISSION_DENIED,
372+
};
373+
}
374+
326375
/**
327376
* CRUD built-in nodes — `get_record` / `create_record` / `update_record` /
328377
* `delete_record`, wired to the runtime data layer (ObjectQL / IDataEngine).
@@ -478,6 +527,10 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
478527
const cfg = parsed.config;
479528
const objectName = cfg.objectName;
480529
if (!objectName) return refuseNode('create_record: objectName required');
530+
// [#21624] A stored-metadata family target is refused before
531+
// anything is resolved or written, under either run identity.
532+
const familyRefusal = storedMetadataWriteRefusal('create_record', objectName);
533+
if (familyRefusal) return familyRefusal;
481534

482535
// #19938 / #11182 ruling D — a CEL value envelope in `fields.*` is
483536
// evaluated; every other value interpolates exactly as before.
@@ -627,6 +680,10 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
627680
const cfg = parsed.config;
628681
const objectName = cfg.objectName;
629682
if (!objectName) return refuseNode('update_record: objectName required');
683+
// [#21624] Before the filter is resolved, so a family target's
684+
// filter is never evaluated, under either run identity.
685+
const familyRefusal = storedMetadataWriteRefusal('update_record', objectName);
686+
if (familyRefusal) return familyRefusal;
630687

631688
// `filters` → `filter` converted at load (ADR-0087 D2); read canonical.
632689
const filterResult = resolveNodeFilter(
@@ -721,6 +778,10 @@ export function registerCrudNodes(engine: AutomationEngine, ctx: PluginContext):
721778
const cfg = parsed.config;
722779
const objectName = cfg.objectName;
723780
if (!objectName) return refuseNode('delete_record: objectName required');
781+
// [#21624] Before the filter is resolved, so a family target's
782+
// filter is never evaluated, under either run identity.
783+
const familyRefusal = storedMetadataWriteRefusal('delete_record', objectName);
784+
if (familyRefusal) return familyRefusal;
724785

725786
// `filters` → `filter` converted at load (ADR-0087 D2); read canonical.
726787
// The highest-stakes of the three: an erased condition here is the

0 commit comments

Comments
 (0)