Repository navigation
Commit 1f04696
fix(trigger-record-change)!: a record-change flow's trigger record carries the credential mask and omits internal fields (#21928)
Fixes #21867
Clause-②: no
## What this changes
Ruling A on #21867 (director's record 5995381726, alignment note
6005796816): mask at the source. `RecordChangeTrigger.buildContext`
(`packages/triggers/trigger-record-change/src/record-change-trigger.ts`)
now projects both roots it hands a flow, `record` and `previous`,
through the one helper `omitInternalFieldsFromWriteResponse`
(`@objectstack/core`,
`packages/core/src/utils/internal-write-response.ts`), with the trigger
object's definition. A credential-class field (every `secret` field, and
every `password` field outside the exempt `managedBy` buckets, per
ADR-0100 and `isMaskedOnReadFieldType`) carries `SECRET_MASK`, or `null`
when unset. A field declared `internal: true` is omitted. `params` is
the same object as `record`, so it inherits the projection.
- **Applied last.** The projection runs after hydration, after
declared-field materialisation and after the decoupling copy, so no
later layer brings a clear value back. It runs in place on the decoupled
copies only, so the engine's `ctx.result` / `ctx.previous` /
`ctx.input`, which are shared with every other binding and hook on the
write, are never touched.
- **The definition is read regardless of ground truth.** Materialisation
is gated on persisted state; the mask is not. A new private
`readObjectDefinition` reads the engine's optional `getObject` accessor.
When the definition cannot be resolved (accessor absent, no answer, or a
throw), the flow still dispatches unmasked, and `readObjectDefinition`
logs that once per object at error through the plugin logger, naming the
object. The bind-time existence probe only warns and still binds;
nothing upstream refuses an unknown object.
- **Downstream inherits it, with no second copy.** The variables map
(`record`, `$record`, `previous`), `SuspendedRun.context`, the persisted
`variables_json` / `context_json`, the run read doors, and the run a
resume rehydrates, in-process and after a restart. ⛔ No mask in
`service-automation` or in the suspended-run store. ⛔ No other variable
is filtered (#7900 stands).
## Premises verified before writing (at `origin/main` `dcb11c2ec9`)
1. **The definition is reachable in `buildContext`.**
`this.engine.getObject` is already read there for materialisation.
Re-check grep: 9 hits in `record-change-trigger.ts`.
2. **The projection is the last overlay.** The last layers are
materialisation, then `decoupleFromEngineState` on both roots, then the
return. The projection sits between the decoupling and the return.
3. **No shipped flow reads a credential-class field off its trigger
record.** The card's grep over `examples/**/*flow*` and
`examples/**/flows/**` returns zero hits (`git grep` exit 1). Control:
the same paths carry `record.FIELD` reads in 4 files, so the zero is not
a dead pattern. The only example object with `password` / `secret`
fields is `showcase_field_zoo`. Its one record-change flow
(`showcase_approver_bindings`, `status: 'draft'`) reads neither field.
4. **Only the trigger's own record enters here.** `get_record` and the
other CRUD nodes (`service-automation/src/builtin/crud-nodes.ts`) read
through `data.find` / `data.findOne`, the engine's generic read path,
which ADR-0100 already masks. Nothing here touches those nodes.
## Pins
-
`packages/triggers/trigger-record-change/src/trigger-record-credential-mask.test.ts`
(unit, fake engine, 13 cases):
- `password` and `secret` carry the mask on `record` and on `previous`,
and the `internal` field is omitted.
- An ordinary field keeps its value, and `params` is the same object as
`record`.
- An unset credential reads `null`.
- The engine's hook objects stay whole.
- Insert events are masked too.
- A `better-auth`-managed `password` keeps the read path's exemption.
- `afterDelete` (record from the prior row) and `beforeUpdate` (payload
over the prior row) are masked on both roots.
- Each of the three unresolved-definition shapes (accessor absent, no
answer, a throw) logs one error naming the object, while the flow runs
on both writes.
- A resolved definition logs no error.
-
`packages/qa/dogfood/test/flow-trigger-record-credential-mask.dogfood.test.ts`.
A real boot: `bootStack` with automation, a file-backed database, the
real crypto provider and the record-change trigger. It uses one object
with an ordinary field, a `password` field, a `secret` field and an
`internal` field, and one `record-after-update` flow that pauses at a
`screen` node. The cases:
- The scene is armed: the engine write result holds the stored values.
- The paused row's `variables_json` and `context_json` carry the mask
for both credential fields and omit the internal field (`record`,
`$record`, `previous`), with no stored credential spelling anywhere in
either column.
- The data door over that row serves the same.
- `GET /automation/:name/runs/:runId` shows the same.
- After the resume, a node reading `record.CREDENTIAL_FIELD` /
`previous.CREDENTIAL_FIELD` stores the mask, while the ordinary field
stores its value.
- The privileged `resolveSecretField` path still returns the plaintext.
- A second suite pauses, stops the kernel, cold-boots a second kernel
over the same file and resumes there. The post-pause node again stores
the mask.
- QA checklist: `automation.paused-run-trigger-record-masked` in
`docs/qa/platform-checklist/areas/automation.json`. This is the item
triage named as missing on the path "approvals and automation — flows
run: errors, pauses and schedules". It covers reading a paused run's
stored state as a non-privileged holder. `automated.ref` names the
dogfood pin, and a `knownGaps` line says the pin reads as the admin.
## Upgrade text
- Changeset `.changeset/21867-flow-trigger-record-credential-mask.md`:
`@objectstack/trigger-record-change` minor, `@objectstack/spec` patch.
It carries the `!` banner, FROM → TO and the one-line handling: a flow
that needs a credential uses a privileged binder, never the trigger
record.
- It names the record-vs-previous credential comparison: a condition
comparing the two sees two equal masks whenever the field is set on both
sides, so a credential change is detected through a privileged binder.
- It carries a "Runs stored before this release" paragraph: paused runs,
and terminal runs that keep a restorable snapshot, created before the
upgrade are resumed, cancelled or purged after upgrading. There is no
migration and no scrub.
- ADR-0087 semantic entry
`packages/spec/src/migrations/entries/semantic/18.flow-trigger-record-credential-masked.ts`,
a sibling of `18.by-id-write-unreadable-row-not-found`. It is registered
through `gen:migration-registry` (`registry.ts`) and declared in the
changeset as `registered flow-trigger-record-credential-masked`.
- `packages/triggers/trigger-record-change/vitest.config.ts`: the alias
moves to the anchored array form and gains `@objectstack/spec/data` and
`@objectstack/core` to source; the `check-test-source-alias` registry
entry for this package drops `@objectstack/core`.
## Verification
Round 1 readings are at head `67ce8a46a2` unless marked. Round 2
readings are in their own block below, at head `4f287e072f`.
- **Ablation.** The two projection calls were replaced via
`scripts/ablation-replace.mjs`, wrap mode, with an EXIT/INT/TERM
restore. On-disk proof: anchor 1 → 0, marker 0 → 1, blob `d0702684cb19`
→ `27a41720a0ab`. The dogfood project aliases
`@objectstack/trigger-record-change` to source, and the plugin is passed
in `extraPlugins` from that import, so no dist hop applies.
- Unit pin: 3 red, 4 green. The four that stay green: ordinary value,
`params` identity, unset reads null, hook objects whole. All four hold
without a mask too.
- Dogfood pin: 5 red, 2 green. The two that stay green: armed scene,
privileged path.
- Restore: blob == HEAD `d0702684cb19`, and `git diff HEAD` is empty.
- **Tests.**
- `@objectstack/trigger-record-change` `pnpm test`: 11 files, 108 tests,
green at `67ce8a46a2`.
- `@objectstack/core` `pnpm test`: 77 files, 2177 tests, green.
- `@objectstack/service-automation` vitest: 173 files, 2112 tests,
green.
- Dogfood pin: 7/7 green, at `48e0b1cc35` (trigger source unchanged
since).
- `@objectstack/spec` `src/migrations`: 3 files, 179 tests, green.
- **Typecheck.**
- `@objectstack/trigger-record-change` `typecheck`, including
`tsconfig.test.json`: green. `--listFiles` counts the new test file
once.
- `@objectstack/dogfood` `typecheck`: green, and it covers the new file.
- `@objectstack/spec` `typecheck` (src, scripts, test layer): green.
- **Gates.**
- `@objectstack/spec` `check:generated`: all 15 artifacts up to date.
- `check:adr-0087-registration`: green. It reads the changeset as
`[BREAKING+bang] registered flow-trigger-record-credential-masked`.
- `check:platform-checklist`: green.
- `dispatch-gates.mjs --ran`: 90 derived, 90 run, 0 NOT-MEASURED, 0
UNRUN.
- **Lint.** The run was narrowed to the 6 changed `.ts` files, under
`eslint --no-inline-config --format json`: 6 files, 0 errors, 0
warnings.
- The population comes from eslint's own config: the two non-code files
(`.changeset/*.md` and `automation.json`) answer "File ignored because
no matching configuration was supplied".
- `--print-config` shows `parserOptions` without `project`, so
type-aware linting is off. This diff cannot move any untouched file's
verdict.
### Round 2, at head `4f287e072f`
`origin/main` was merged in as a merge commit (`baa4b2fe6c`; the branch
was 9 behind).
- **Build and tests.**
- Closure build `pnpm --workspace-concurrency=2 --filter
'@objectstack/trigger-record-change...' build`: exit 0.
- `@objectstack/trigger-record-change` `pnpm test`: 11 files, 114 tests,
green. The mask file has 13 cases.
- `@objectstack/trigger-record-change` `typecheck` (`tsc --noEmit && tsc
--noEmit -p tsconfig.test.json`): exit 0 for both.
- **Ablation 1, the core alias resolves to source.** Via
`scripts/ablation-replace.mjs`, an early return was planted in
`omitInternalFieldsFromWriteResponse`
(`packages/core/src/utils/internal-write-response.ts`), with core `dist`
not rebuilt (marker: 0 hits in `packages/core/dist`). Landed: anchor 1 →
0, blob `2a6a48c04fdb` → `d51283c8f5e6`. Result: 5 red, 8 green; the red
ones are the masking cases, the new `afterDelete` and `beforeUpdate`
included. Restore: blob == HEAD `2a6a48c04fdb`, `git diff HEAD` empty. A
first attempt was refused by the tool as a no-op (the replacement
contained the anchor); it measured nothing and was redone with a
non-overlapping replacement.
- **Ablation 2, the log pin can fail.** The error branch's condition was
replaced with `false`. Landed: anchor 1 → 0. Result: 3 red (the absent,
no-answer and throw cases), 10 green. Restore: blob == HEAD
`04e3ca86825f`, `git diff HEAD` empty.
- **Gates, each exit 0.** `check:adr-0087-registration` (reads
`[BREAKING+bang] registered flow-trigger-record-credential-masked`;
`--self-test` 441 assertions), `check-adr-0087-registration --base
origin/main`, `check-changeset-no-major --base origin/main`,
`check-empty-changeset --base origin/main`,
`check:changeset-gate-self-tests`, `check:test-source-alias` (73
packages with tests scanned, 60 registered), `check:nul-bytes`,
`check-scripts-symbol-anchors`, `check-published-list-mirrors`,
`check:cross-package-test-inputs`, `check:doc-authoring`,
`check:issue-citations`, `check:logger-receiver-detach`,
`check-changeset-fixed`, `check:published-files`.
- `@objectstack/spec` `check:generated` after the main merge: all 15
generated artifacts up to date, against the spec `dist` built
post-merge.
- NOT MEASURED: `check:console-injection`. It skipped, because there is
no `packages/console/dist` in this worktree.
- The rest of the `dispatch-gates` derivation (109 commands over the
whole PR diff, mostly round-1 spec and dogfood families) was not re-run
this round; CI owns it.
- **Lint.** Narrowed to the 4 files changed this round
(`record-change-trigger.ts`, `trigger-record-credential-mask.test.ts`,
`vitest.config.ts`, `scripts/check-test-source-alias.mjs`), under
`eslint --no-inline-config --format json`: 4 files, 0 errors, 0
warnings. All 4 are in eslint's own config, per `--print-config`, which
also shows `parserOptions.project` and `projectService` undefined, so
type-aware linting is off and this diff cannot move any untouched file's
verdict.
## Acceptance notes
- The claim's file surface names
`packages/triggers/trigger-record-change/src`. This PR also touches that
package's `vitest.config.ts` (the alias above) and adds one dogfood test
file under `packages/qa/dogfood/test/`, as the dispatch asked. Round 2
also touches `scripts/check-test-source-alias.mjs`, a registry narrowing
only (this package's entry drops `@objectstack/core`).
- During the second full gate pass,
`packages/plugins/plugin-approvals/dist` and
`packages/plugins/plugin-auth/dist` were found without `.d.ts` (written
mid-pass). `check:dts-closure` and `check:dual-build-cjs-loads` went red
as a result. A rebuild of those two packages restored them, and both
gates read green. Neither package is in this diff. Which step wrote them
was not established.
- Carrier: none; noted here only, not filed. In `buildContext`, the
materialisation read of `getObject` (gated on ground truth) is not
wrapped in try/catch. A `getObject` that throws therefore fails the
dispatch before the mask runs, and the handler logs "execution failed".
So `readObjectDefinition`'s throw branch is reachable only on an update
or delete with no prior row. This behaviour predates the PR and was left
untouched, because the dispatch said dispatch behaviour must not change.
No public entry point is shown to throw from `getObject`.
- Of the three operator actions for runs stored before this release,
purging is the only one that leaves no clear value behind; resuming an
old paused run can still write its clear values into the run's step log.
A follow-up edit to the changeset should list purge first.
- `48c162ed06` ports the three dogfood test-infra files of open PR
#21935 (`packages/qa/dogfood/test/per-file-cwd.setup.ts`,
`per-file-cwd.global-setup.ts`, `packages/qa/dogfood/vitest.config.ts`),
byte-identical, to clear the `PM dispatch-gates self-test` red that
`main` has carried since #21919. It is a no-op once #21935 lands.
---
_Generated by [Claude
Code](https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 412d7dd commit 1f04696
9 files changed
Lines changed: 855 additions & 8 deletions
File tree
- .changeset
- docs/qa/platform-checklist/areas
- packages
- qa/dogfood/test
- spec/src/migrations
- entries/semantic
- triggers/trigger-record-change
- src
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
944 | 944 | | |
945 | 945 | | |
946 | 946 | | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
| 997 | + | |
| 998 | + | |
| 999 | + | |
| 1000 | + | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
| 1004 | + | |
| 1005 | + | |
| 1006 | + | |
| 1007 | + | |
| 1008 | + | |
| 1009 | + | |
| 1010 | + | |
| 1011 | + | |
| 1012 | + | |
| 1013 | + | |
| 1014 | + | |
| 1015 | + | |
| 1016 | + | |
| 1017 | + | |
| 1018 | + | |
| 1019 | + | |
| 1020 | + | |
| 1021 | + | |
| 1022 | + | |
| 1023 | + | |
| 1024 | + | |
| 1025 | + | |
| 1026 | + | |
| 1027 | + | |
| 1028 | + | |
| 1029 | + | |
| 1030 | + | |
| 1031 | + | |
| 1032 | + | |
| 1033 | + | |
| 1034 | + | |
| 1035 | + | |
| 1036 | + | |
| 1037 | + | |
| 1038 | + | |
947 | 1039 | | |
948 | 1040 | | |
949 | 1041 | | |
| |||
0 commit comments