Skip to content

fix(data): record-change payloads apply the write-response credential mask and internal-field omission - #21866

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-21830-record-change-credential
Oct 5, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-21830-record-change-credential

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #21830

Clause-②: yes (widening)

Record-change payloads apply the same credential mask and internal-field omission as write responses.

What changed

One rule, one helper: omitInternalFieldsFromWriteResponse (@objectstack/core, the helper PR #21816 landed for write responses). No second copy of the rule anywhere.

  • Engine record-change publish (packages/objectql/src/engine.ts, publishDataEvent). The after and changes bodies of data.record.created / data.record.updated are projected through the helper on a fresh shallow copy: credential-class fields carry SECRET_MASK (or null when unset), internal: true fields are omitted. The engine's own write result is untouched, so privileged in-process callers are unchanged.
  • Defence in depth at the downstream producers that store or forward a record body, each through the same helper or its collectors:
    • approval request snapshot (plugin-approvals, payload_json when the request is opened);
    • outbound webhook body and its stored delivery row (plugin-webhooks auto-enqueuer, after and changes);
    • knowledge index documents (service-knowledge recordToDocument takes the object definition as an optional fourth argument and skips credential-class and internal fields, under '*' and when named explicitly).
  • The audit trail already masked these fields and is unchanged. No accept set changes; the one public-surface change is additive (see Review round 1).
  • Changeset: .changeset/record-change-payload-credential-mask.md (minor for @objectstack/service-knowledge, patch for the other three).

Verification (head d6e0eae, after merging origin/main)

  • Dependency closure build of the four touched packages: VERDICT command-exit 0.
  • @objectstack/objectql full suite: Test Files 376 passed (376), Tests 7486 passed (7486).
  • @objectstack/plugin-approvals: 60 files / 895 tests passed. @objectstack/plugin-webhooks: 14 / 163 passed. @objectstack/service-knowledge: 4 / 49 passed.
  • typecheck for objectql, plugin-approvals, plugin-webhooks: green (check:test-typecheck: OK). service-knowledge has no typecheck script.
  • Dogfood, real boot: test/approval-snapshot-credential-field.dogfood.test.ts 1 passed, after a build of the dogfood dependency closure.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands derived 75 families; all 75 run, all exit 0 (plus check-nul-bytes); --ran reconciliation: 75 run, 0 NOT-MEASURED, a derived zero with exit codes recorded.
  • Ablation (one-shot, via scripts/ablation-replace.mjs, from the committed state): removing the helper call in the engine's event-body projection turns src/engine-realtime-credential-mask.test.ts red (3 failed, 1 passed); restore proven by blob hash equal to HEAD (48b8210911cc) and an empty git diff HEAD.
  • Not run locally, declared to CI: repo-wide lint and the wide-population gate families.

Acceptance notes

  • Payload projections are idempotent over an already-projected body, so the engine projection and the downstream defence-in-depth layers compose.
  • A producer whose engine exposes no getSchema (or an unregistered object) projects nothing, matching the helper's own posture; the engine-side projection still applies upstream.

Generated by Claude Code

Review round 1 (PM seat)

  • @objectstack/service-knowledge is now minor: recordToDocument, a published export, gains an optional fourth argument (the object definition); three-argument calls behave as before.
  • The outbound webhook projection also covers before (no producer fills it today).
  • The changeset now says receivers see masked values, and that rows written before this change are not rewritten (reindexing a knowledge source refreshes its documents).
  • Head c87404e8c4: @objectstack/plugin-webhooks 163/163; the changeset gates green against origin/main.

@github-actions github-actions Bot added the size/l label Oct 5, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/objectql, @objectstack/plugin-approvals, @objectstack/plugin-webhooks, @objectstack/service-knowledge, touching 13 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ai/knowledge-rag.mdx (via KnowledgeService (symbol, a top-level class))
  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class))
  • content/docs/automation/webhooks.mdx (via AutoEnqueuer (symbol, a top-level class))
  • content/docs/protocol/kernel/realtime-protocol.mdx (via publishDataEvent (symbol, a method of class ObjectQL))
  • content/docs/protocol/knowledge.mdx (via KnowledgeService (symbol, a top-level class), reindexSource (symbol, a method of class KnowledgeService))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9f9510f25e6aa65aa61ce3effb42706fabcab92e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from dcb3d872ea9f236c41019902ece3e94046cf63f3 — the merge of head c87404e8c43e7787353d6b79b014896a5dbc0cee into base 9f9510f25e6aa65aa61ce3effb42706fabcab92e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin dcb3d872ea9f236c41019902ece3e94046cf63f3 && git checkout dcb3d872ea9f236c41019902ece3e94046cf63f3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9f9510f25e6aa65aa61ce3effb42706fabcab92e c87404e8c43e7787353d6b79b014896a5dbc0cee && git checkout -B drift-repro 9f9510f25e6aa65aa61ce3effb42706fabcab92e && git merge --no-ff c87404e8c43e7787353d6b79b014896a5dbc0cee

node scripts/docs-audit/affected-docs.mjs --json 9f9510f25e6aa65aa61ce3effb42706fabcab92e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 9f9510f25e6aa65aa61ce3effb42706fabcab92e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: c87404e8c43e7787353d6b79b014896a5dbc0cee
Local-runs: none

① Derived judgments

  • Accept sets: none move, named right. No zod schema, no DataEventSchema shape and no metadata accept set is touched. The event-body change is to values only: what gets masked or left out is decided by the @objectstack/core collectors already on main, and those ask isMaskedOnReadFieldType from @objectstack/spec/data. No second copy of the type set exists (a triage requirement, met).
  • Engine record-change publish: right. The after and changes bodies of single-record created and updated events go through omitInternalFieldsFromWriteResponse, the one helper from fix(core): mask credential-class fields on every write response #21816, on a fresh shallow copy. So the engine's own write result is not mutated. The deleted event carries no record body, so nothing there is left unprojected. Aggregate data.records.* events carry counts only, so they are out of the class.
  • Defence-in-depth producers: right, all through the same helper or its collectors. These are the approval snapshot (the only payload_json write), the outbound webhook body and its stored delivery row (before, after and changes, with the shared event left untouched), and knowledge documents (fields are skipped rather than indexed as the mask, under '*', when named explicitly, and for title). Each is idempotent over an already-projected body, so the layers compose. The no-schema posture (project nothing) matches the helper's own.
  • Public surface: one additive change, named right. recordToDocument (a published export of @objectstack/service-knowledge) gains an optional fourth argument. Three-argument calls behave as before. There are no new imports outside existing workspace:* dependencies, and all three core symbols used are exported from packages/core/src/index.ts.
  • Observable behaviour change, declared. Realtime and webhook receivers now get SECRET_MASK (or null when unset) for credential-class fields, and no key for internal fields. This is the same answer a read of the row already gives. The changeset says so, and also says that rows already stored are not rewritten.
  • Pins: present, per the triage pins. Each consumer class reads the masked value. A privileged in-process write result is shown unchanged (engine test). The input record is shown untouched (approvals and webhooks). A no-schema case is included. There is also a real-boot dogfood pin. The dev's ablation of the engine projection turned the engine pin red.

② Semver level

  • Changeset matches what the diff publishes. @objectstack/service-knowledge is minor, for the additive optional argument on a published export. @objectstack/objectql, @objectstack/plugin-approvals and @objectstack/plugin-webhooks are patch: these are security fixes that bring the event and stored bodies into line with the read and write-response contract, with no signature or schema change.
  • Clause-②: yes (widening). Correct. It is carried by the PR body, the changeset and the PM seat's claim correction, which supersedes the claim's original no.
  • Nit, not blocking. The PR body's original "What changed" section still says "patch on the four packages" and "No accept set or public schema change". The "Review round 1" section and the changeset supersede that, and the changeset itself is accurate.

③ Boundary flags

  • open_questions: empty. out_of_scope_findings: empty. No dev flag was raised.
  • Boundary: none moves. The card states it is not a decision card, and the diff only brings non-exposure in line with the existing read boundary. Nothing needs escalating.
  • Withheld detail: the record is kept at class level (RUNNER rule 2). No consumer names, reproduction or mechanism beyond what the card and PR already state publicly.
  • CI incomplete on this head. At review time these were still in progress: Build Core, Test Core 1 to 6 of 6, Dogfood Regression Gate 1 to 3 of 3, Dogfood Verify CLI, Temporal Conformance, Lint & Repo Gates, all four Type Check jobs, and the re-run of Check Changeset. Already successful: the earlier Check Changeset run, both single-claim gates, the Part-of gate and filter. The verdict is on the diff; landing still waits on those runs going green.

Implemented-by: claude/issue-21830-record-change-credential
Reviewed-by: session_018zT8d8NpiQ1ExhuNd5TxY6

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 5, 2026 12:06
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 5, 2026 12:06
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 5, 2026
Merged via the queue into main with commit 568dc0b Oct 5, 2026
58 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21830-record-change-credential branch October 5, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants