fix(trigger-record-change)!: a record-change flow's trigger record carries the credential mask and omits internal fields - #21928
Conversation
…us values carry the credential mask and omit internal fields Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…igger record Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…masked, hot and after a cold boot Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…te fixture Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…r the masked flow trigger record Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…on checklist Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…r the mask pin Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 139 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 56199bee56e35a5e97d1257b412f3d33414b1ace && git checkout 56199bee56e35a5e97d1257b412f3d33414b1ace
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9dce635337c2cc42a4149aa49289ad77d172363d 48c162ed0680a540fbef54b2beb27cec619dde2b && git checkout -B drift-repro 9dce635337c2cc42a4149aa49289ad77d172363d && git merge --no-ff 48c162ed0680a540fbef54b2beb27cec619dde2b
node scripts/docs-audit/affected-docs.mjs --json 9dce635337c2cc42a4149aa49289ad77d172363d
|
Contract reviewServed-tier: Inputs: card #21867 (body, triage 5993882228, director ruling 5995381726 letter A, PM note 6005739672, alignment note 6005796816, claim 6005816377, os-dev-report 6007440350); PR #21928 body, its 8-file list, and the net diff ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
… is logged at error, once per object The mask in buildContext needs the object's definition. The comment claimed an unknown object is refused upstream; the bind-time probe only warns and still binds, so the comment is corrected and the unresolved case (accessor absent, no answer, or a throw) now logs at error through the plugin's logger, naming the object. Dispatch is unchanged. Pins the log and adds afterDelete and beforeUpdate mask cases. Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
… mask helper is tested from the checkout Narrows the KNOWN_UNALIASED_TEST_IMPORTS entry for this package to the three dependencies still resolved through dist. Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
…d the runs stored before the release Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Inputs: card #21867 (body; triage 5993882228; director ruling 5995381726, letter A; PM note 6005739672; alignment note 6005796816; claim 6005816377; round-1 os-dev-report 6007440350; round-2 os-dev-report 6007718665). PR #21928: its body, its 9-file list, and the net diff ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
main is red on the dispatch-gates self-test since the per-file cwd setup landed: its mkdtempSync base is not readable by the scratch-dir scan. This ports the same three-file change as the open fix PR, so this branch's lint lane goes green; it is a no-op once main carries that fix. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
|
The fix is open as #21935. I ported the same three files ( Generated by Claude Code |
Contract reviewServed-tier: Inputs: card #21867 (body; triage 5993882228; director ruling 5995381726, letter A; PM note 6005739672; alignment note 6005796816; claim 6005816377; os-dev-reports 6007440350 and 6007718665). PR #21928: its body, its 12-file list, its comments (the earlier records 6007494437 and 6007858445, and the port note 6008303479), the commit diff ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
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,recordandprevious, through the one helperomitInternalFieldsFromWriteResponse(@objectstack/core,packages/core/src/utils/internal-write-response.ts), with the trigger object's definition. A credential-class field (everysecretfield, and everypasswordfield outside the exemptmanagedBybuckets, per ADR-0100 andisMaskedOnReadFieldType) carriesSECRET_MASK, ornullwhen unset. A field declaredinternal: trueis omitted.paramsis the same object asrecord, so it inherits the projection.ctx.result/ctx.previous/ctx.input, which are shared with every other binding and hook on the write, are never touched.readObjectDefinitionreads the engine's optionalgetObjectaccessor. When the definition cannot be resolved (accessor absent, no answer, or a throw), the flow still dispatches unmasked, andreadObjectDefinitionlogs 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.record,$record,previous),SuspendedRun.context, the persistedvariables_json/context_json, the run read doors, and the run a resume rehydrates, in-process and after a restart. ⛔ No mask inservice-automationor in the suspended-run store. ⛔ No other variable is filtered (finding: the whole /automation read domain is gated only by "authenticated" — run-detail returns the triggering record's fields without that record's own FLS #7900 stands).Premises verified before writing (at
origin/maindcb11c2ec9)buildContext.this.engine.getObjectis already read there for materialisation. Re-check grep: 9 hits inrecord-change-trigger.ts.decoupleFromEngineStateon both roots, then the return. The projection sits between the decoupling and the return.examples/**/*flow*andexamples/**/flows/**returns zero hits (git grepexit 1). Control: the same paths carryrecord.FIELDreads in 4 files, so the zero is not a dead pattern. The only example object withpassword/secretfields isshowcase_field_zoo. Its one record-change flow (showcase_approver_bindings,status: 'draft') reads neither field.get_recordand the other CRUD nodes (service-automation/src/builtin/crud-nodes.ts) read throughdata.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):passwordandsecretcarry the mask onrecordand onprevious, and theinternalfield is omitted.paramsis the same object asrecord.null.better-auth-managedpasswordkeeps the read path's exemption.afterDelete(record from the prior row) andbeforeUpdate(payload over the prior row) are masked on both roots.packages/qa/dogfood/test/flow-trigger-record-credential-mask.dogfood.test.ts. A real boot:bootStackwith automation, a file-backed database, the real crypto provider and the record-change trigger. It uses one object with an ordinary field, apasswordfield, asecretfield and aninternalfield, and onerecord-after-updateflow that pauses at ascreennode. The cases:variables_jsonandcontext_jsoncarry the mask for both credential fields and omit the internal field (record,$record,previous), with no stored credential spelling anywhere in either column.GET /automation/:name/runs/:runIdshows the same.record.CREDENTIAL_FIELD/previous.CREDENTIAL_FIELDstores the mask, while the ordinary field stores its value.resolveSecretFieldpath still returns the plaintext.automation.paused-run-trigger-record-maskedindocs/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.refnames the dogfood pin, and aknownGapsline says the pin reads as the admin.Upgrade text
.changeset/21867-flow-trigger-record-credential-mask.md:@objectstack/trigger-record-changeminor,@objectstack/specpatch. 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.packages/spec/src/migrations/entries/semantic/18.flow-trigger-record-credential-masked.ts, a sibling of18.by-id-write-unreadable-row-not-found. It is registered throughgen:migration-registry(registry.ts) and declared in the changeset asregistered 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/dataand@objectstack/coreto source; thecheck-test-source-aliasregistry entry for this package drops@objectstack/core.Verification
Round 1 readings are at head
67ce8a46a2unless marked. Round 2 readings are in their own block below, at head4f287e072f.scripts/ablation-replace.mjs, wrap mode, with an EXIT/INT/TERM restore. On-disk proof: anchor 1 → 0, marker 0 → 1, blobd0702684cb19→27a41720a0ab. The dogfood project aliases@objectstack/trigger-record-changeto source, and the plugin is passed inextraPluginsfrom that import, so no dist hop applies.paramsidentity, unset reads null, hook objects whole. All four hold without a mask too.d0702684cb19, andgit diff HEADis empty.@objectstack/trigger-record-changepnpm test: 11 files, 108 tests, green at67ce8a46a2.@objectstack/corepnpm test: 77 files, 2177 tests, green.@objectstack/service-automationvitest: 173 files, 2112 tests, green.48e0b1cc35(trigger source unchanged since).@objectstack/specsrc/migrations: 3 files, 179 tests, green.@objectstack/trigger-record-changetypecheck, includingtsconfig.test.json: green.--listFilescounts the new test file once.@objectstack/dogfoodtypecheck: green, and it covers the new file.@objectstack/spectypecheck(src, scripts, test layer): green.@objectstack/speccheck: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..tsfiles, undereslint --no-inline-config --format json: 6 files, 0 errors, 0 warnings..changeset/*.mdandautomation.json) answer "File ignored because no matching configuration was supplied".--print-configshowsparserOptionswithoutproject, so type-aware linting is off. This diff cannot move any untouched file's verdict.Round 2, at head
4f287e072forigin/mainwas merged in as a merge commit (baa4b2fe6c; the branch was 9 behind).pnpm --workspace-concurrency=2 --filter '@objectstack/trigger-record-change...' build: exit 0.@objectstack/trigger-record-changepnpm test: 11 files, 114 tests, green. The mask file has 13 cases.@objectstack/trigger-record-changetypecheck(tsc --noEmit && tsc --noEmit -p tsconfig.test.json): exit 0 for both.scripts/ablation-replace.mjs, an early return was planted inomitInternalFieldsFromWriteResponse(packages/core/src/utils/internal-write-response.ts), with coredistnot rebuilt (marker: 0 hits inpackages/core/dist). Landed: anchor 1 → 0, blob2a6a48c04fdb→d51283c8f5e6. Result: 5 red, 8 green; the red ones are the masking cases, the newafterDeleteandbeforeUpdateincluded. Restore: blob == HEAD2a6a48c04fdb,git diff HEADempty. 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.false. Landed: anchor 1 → 0. Result: 3 red (the absent, no-answer and throw cases), 10 green. Restore: blob == HEAD04e3ca86825f,git diff HEADempty.check:adr-0087-registration(reads[BREAKING+bang] registered flow-trigger-record-credential-masked;--self-test441 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/speccheck:generatedafter the main merge: all 15 generated artifacts up to date, against the specdistbuilt post-merge.check:console-injection. It skipped, because there is nopackages/console/distin this worktree.dispatch-gatesderivation (109 commands over the whole PR diff, mostly round-1 spec and dogfood families) was not re-run this round; CI owns it.record-change-trigger.ts,trigger-record-credential-mask.test.ts,vitest.config.ts,scripts/check-test-source-alias.mjs), undereslint --no-inline-config --format json: 4 files, 0 errors, 0 warnings. All 4 are in eslint's own config, per--print-config, which also showsparserOptions.projectandprojectServiceundefined, so type-aware linting is off and this diff cannot move any untouched file's verdict.Acceptance notes
packages/triggers/trigger-record-change/src. This PR also touches that package'svitest.config.ts(the alias above) and adds one dogfood test file underpackages/qa/dogfood/test/, as the dispatch asked. Round 2 also touchesscripts/check-test-source-alias.mjs, a registry narrowing only (this package's entry drops@objectstack/core).packages/plugins/plugin-approvals/distandpackages/plugins/plugin-auth/distwere found without.d.ts(written mid-pass).check:dts-closureandcheck:dual-build-cjs-loadswent 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.buildContext, the materialisation read ofgetObject(gated on ground truth) is not wrapped in try/catch. AgetObjectthat throws therefore fails the dispatch before the mask runs, and the handler logs "execution failed". SoreadObjectDefinition'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 fromgetObject.48c162ed06ports the three dogfood test-infra files of open PR test(dogfood): each file's temporary cwd is created from a base the scratch-dir scan can read #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 thePM dispatch-gates self-testred thatmainhas carried since test(dogfood): every test file runs in its own temporary working directory #21919. It is a no-op once test(dogfood): each file's temporary cwd is created from a base the scratch-dir scan can read #21935 lands.Generated by Claude Code