Skip to content

fix(metadata-protocol): a seed replay no longer rewrites every seeded row that carries a password or secret field on every boot - #24696

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24624-replay-masked-field
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24624-replay-masked-field

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24624

Clause-②: no (a replay comparison: no accept set, served body or masking verdict moves; the loader takes no new read and never sees a stored credential)

What this changes

Ruling 6278934212 on #24624 records letter B of analysis 6275762852 (the maintainer, verbatim: 「同意」): a masked field never blocks the replay's skip.

  • packages/metadata-protocol/src/seed-loader.ts: the replay read (engine.find, isSystem: true) serves a set secret field, and a set password field on an object not managed by better-auth, as SECRET_MASK (ADR-0100). isNoOpReplay compared the seed's plaintext with that mask, read the row as changed, and rewrote it on every boot. It now skips a field when all three hold: the object declares the field masked on read, the existing row read back SECRET_MASK, and the seed sets a value.
  • isNoOpReplay stays a pure predicate, because decideWriteAction asks it up to three times per record (the refusal check, the early signal and the decision). The skip arms carry the uncompared fields, and the load records them only where a skip is counted.
  • One info line per load, when a replay skipped at least one row without comparing a credential. It names every such field, the rule, and both remedies (change another field of the row, or rebuild the database). Text from the real-engine reading below:

    [SeedLoader] 1 seeded row(s) were left unchanged without comparing their credential field(s) showcase_field_zoo.f_password: a secret field, and a password field on an object not managed by better-auth, is masked on read, so a replay cannot see whether the seed changed a value that is already set and never rewrites a row for that alone. A seed change to only such a field is not applied on replay: change another field of the same row too (the rewrite then writes the seed's value), or rebuild the database.

  • What this PR does not do: take a new unmasked read, change ObjectQL.maskSecretFields or collectMaskedReadFields, add an export, edit an ADR, or add a dependency.

How the loader knows a field is masked (dispatch mechanism assumption 2)

  • (b) collectMaskedReadFields from @objectstack/objectql: impossible. packages/objectql/package.json already lists @objectstack/metadata-protocol under dependencies (workspace:*), so the import would create a dependency cycle.
  • (a) the mask alone (existing[key] === SECRET_MASK) fixes the showcase. But it would also skip any non-credential field that happens to store the eight-bullet string, so a seed change to that field would be dropped silently. Ablation leg 2 below measures this: the pin a field the declaration does not mask keeps comparing, even when it holds the mask's text goes red.
  • Used: both, each read from its single source. The field check asks isMaskedOnReadFieldType(type, managedBy) from @objectstack/spec/data. That is the declaration collectMaskedReadFields itself asks, and @objectstack/spec is already a dependency of this package. The value check reads SECRET_MASK from the same module. The loader contains no copy of the "secret, or password on a non-better-auth object" rule.

One reading of the ruling for the seat to confirm: presence still counts

The ruling's execution text says isNoOpReplay "skips the fields collectMaskedReadFields collects". This PR skips such a field only while its value is set. The mask preserves presence: an unset credential reads back null, never the mask (SECRET_MASK's docblock, ADR-0100). So a null read is a stored fact the replay can compare:

  • a seed that sets a credential on a row that has none is written. Under the literal reading it would never be written unless another field changed too;
  • a seed that sets a credential to null over a set one is written, which clears it.

Neither case churns, because the next replay reads the same presence and skips. Ablation leg 3 swaps in the literal reading (return comparands.masked.has(key);), and it turns only the presence pin red. If the seat reads the ruling literally, that one-line change is the entire difference.

Notice level: info, and what that costs

  • Why info: this skip is the declared, ruled outcome, and it fires on every healthy replay of a seed that carries a credential (the stock showcase, every boot). The precedent is filterByEnv's skip line in the same file, which logs at info "rather than crying wolf on every production boot". The official examples also boot with zero warnings (examples/app-showcase/test/no-startup-warnings.test.ts). Nothing the system claims to hold is lost, because the row keeps its credential, so this is not AGENTS.md's durability class.
  • The cost, measured: the CLI's default kernel level is warn (packages/cli/src/utils/log-level.ts). The boot-quiet window also drops in-window info whatever --log-level says (BOOT_DIAGNOSTIC_FLOOR = 'warn', packages/cli/src/utils/boot-log-capture.ts). So on an os dev or os serve boot the line is not on screen. It reaches a host logging at info outside that window: hot reload, package apply, draft publish, marketplace install and per-organization replay. If the seat wants the line visible at boot, it is a one-word change to warn, at the price of a warning on every showcase boot.

Tests (all at a947341b91, after merging origin/main 92ce3af404)

  • New pins in packages/metadata-protocol/src/seed-loader-replay.test.ts, block seed replay — a credential the read path masks never blocks the skip. The engine double wraps the existing faithful engine. It stores a password verbatim and a secret as an opaque ref, stamps updated_at on every write, and masks on read through isMaskedOnReadFieldType (returning copies). Both write paths are covered: batched decideWriteAction, and sequential writeRecord through a self-referencing object.
    • an unchanged replay issues no update, and the store, updated_at included, is deep-equal (both paths × upsert/update);
    • a credential-only change (password and secret) is not rewritten, and the load emits exactly one notice, at info, naming every uncompared field;
    • a change to another field rewrites the row: the stored password becomes the seed's new value, and the secret update carries the new plaintext;
    • setting an unset credential, and clearing a set one, are both written;
    • a field the declaration does not mask keeps comparing even when it holds the mask's text;
    • no notice when no skipped row left a credential uncompared.
  • Red first. Before the fix (bfc1539fe0 source), 10 of the new pins failed: every row carrying a credential was rewritten (3 of 4 rows updated on an unchanged replay).
  • pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2: Test Files 249 passed | 3 skipped (252), Tests 20793 passed | 19 skipped (20812), verify-lock VERDICT command-exit 0.
  • pnpm --filter @objectstack/metadata-protocol typecheck: exit 0. tsc --listFiles includes seed-loader-replay.test.ts.
  • Ablation, one-shot, through scripts/ablation-replace.mjs (anchor hits 1, mutation proven on disk by count and blob, restored to the HEAD blob 3963a4b4 with git diff HEAD empty after each leg). The test imports ./seed-loader.js from src, so no dist/ sits on the path. Expected directions were declared first, and all three held:
    1. delete the mask branch in isNoOpReplay: 10 failed | 15 passed (the replay, credential-only, other-field and presence pins);
    2. value-only (return stored === SECRET_MASK && seedValue != null;, i.e. option (a) alone): 1 failed | 24 passed (the literal-bullets pin only);
    3. declaration-only (return comparands.masked.has(key);, the literal reading): 2 failed | 23 passed (the presence pin, both paths).

Real-engine reading (scratch harness, not committed)

Setup: real ObjectQL with SqlDriver (better-sqlite3) on one SQLite file. Every examples/app-showcase object definition, plus sys_business_unit, and every showcase seed dataset (19 datasets, 132 rows). One process per "boot"; the loader runs from this branch's source.

run inserted updated skipped Specimen — Full stored f_password its updated_at notices
boot 1 (fresh file) 132 0 0 hunter2 (plaintext at rest; engine.find reads the mask) …23:17:00.189Z 0
boot 2 (same seed) 0 0 132 hunter2 unchanged 1
password-only change (hunter3) 0 0 132 hunter2 unchanged 1
hunter3 + f_phone changed 0 1 131 hunter3 moved 0
same seed, mask branch ablated (the pre-fix comparison) 0 1 (Specimen — Full) 131 — moved 0
same seed, restored 0 0 132 — unchanged 1

This confirms dispatch mechanism assumption 1 on the real engine: f_password is stored as plaintext, and the seed's own system-context read returns SECRET_MASK.

Gates

  • Derived list, run locally at a947341b91: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 64 families, with the change set taken from git by the tool. All 64 exited 0, each exit captured before any pipe.
  • Reconciliation: --ran reports 64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN, a derived zero because every line carries its exit code. On the pre-merge head 20abbaf71d, check:dual-build-cjs-loads printed PREREQUISITE NOT MET (not every package had a dist/). At a947341b91 it measured green: 111 require entry points across 69 packages.
  • Not run locally:
    • node scripts/check-platform-checklist.mjs --changed-from origin/main: no export was renamed or removed.
    • test:repo: the diff touches no repo-project test. CI's Test Core runs it.
  • CI-owned families: read from this PR's check-runs, not re-run here.

Acceptance notes


Generated by Claude Code

…ead path masks

A secret field, and a password field on an object not managed by
better-auth, reads back as SECRET_MASK through engine.find, so the
replay compared the seed's plaintext with the mask and rewrote every
seeded row carrying one on every boot. Such a field no longer blocks
the skip while a value is set; setting or clearing one is still a
change, and a load that skipped a row without comparing one says so in
one info line naming the fields and the remedy.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 13 documentable anchor(s).

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

  • content/docs/data-modeling/validation.mdx (via SeedLoaderService (symbol, a top-level class))
  • content/docs/protocol/objectql/state-machine.mdx (via SeedLoaderService (symbol, a top-level class))

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

  • content/docs/releases/v17/17-1.mdx (via SeedLoaderService (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
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 11 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 045dae8f23d21e7631e425bb851cad5ab2bc26fd → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 89b2f743547f98c1ccca78cc4bf22f644d351ab6 — the merge of head a947341b91360e265db6cf549f2225e1c8615c3f into base 045dae8f23d21e7631e425bb851cad5ab2bc26fd, 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 89b2f743547f98c1ccca78cc4bf22f644d351ab6 && git checkout 89b2f743547f98c1ccca78cc4bf22f644d351ab6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 045dae8f23d21e7631e425bb851cad5ab2bc26fd a947341b91360e265db6cf549f2225e1c8615c3f && git checkout -B drift-repro 045dae8f23d21e7631e425bb851cad5ab2bc26fd && git merge --no-ff a947341b91360e265db6cf549f2225e1c8615c3f

node scripts/docs-audit/affected-docs.mjs --json 045dae8f23d21e7631e425bb851cad5ab2bc26fd

⚠️ 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 045dae8f23d21e7631e425bb851cad5ab2bc26fd → 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

席位核对 · ACCEPT · domain:engine seat 2 · session_01AHeUBbe6S3JRawcpbLejXD · 2026-10-20T00:21Z

Reviewed against GitHub on head a947341b91, not against the report.

  • Form: draft into main. The body's first line is Fixes #24624, and Clause-②: no (...) sits at a line start. A closing-keyword scan of the whole body finds one hit, Fixes #24624.
  • Scope: 3 files, +441 / −22: packages/metadata-protocol/src/seed-loader.ts, seed-loader-replay.test.ts and .changeset/24624-replay-masked-field.md (patch on @objectstack/metadata-protocol). All three are inside claim 6279045139's surface. check-governed-merges --pr 24696: not governed, 463 changed lines.
  • Ruling 6278934212 (letter B), checked clause by clause against the diff:
    • A field the object declares masked on read never blocks the skip. Which fields those are comes from isMaskedOnReadFieldType (@objectstack/spec/data, masked-field-types.ts:116), the predicate objectql's secret-fields.ts:40 imports, so no second copy of the masking rule exists. collectMaskedReadFields itself is out of reach (@objectstack/objectql depends on this package), as the report says.
    • One info line per load names the fields, the rule and both remedies (change another field, or rebuild the database). It is filled only where a skip is counted, so isNoOpReplay stays pure.
    • Both directions are pinned on both write paths (batched decideWriteAction, sequential writeRecord). A credential-only change is not rewritten and prints one line. A change to another field rewrites the row with the new credential. A load with no such skip prints nothing.
    • ⛔ clauses hold: no new read, maskSecretFields / collectMaskedReadFields are untouched, no new export, no ADR edit.
  • The report's open questions, answered by this seat (execution parameters inside ruling B; neither moves what B decided):
    1. Presence is still compared (the PR's reading, A). B's subject is a value the replay cannot read back. An unset credential reads null, not the mask, so set-versus-unset is a stored fact the replay reads exactly, and setting or clearing a credential is written. That follows the loader's "any doubt reads as changed" rule. The skip still never churns: after the first write, the row reads the mask and is skipped. The literal reading (B) would drop two changes the loader can see.
    2. info stays (the PR's choice, A). The line fires on every healthy replay of a seed that carries a credential, where nothing is lost, so warn would put a warning on every showcase boot (examples/app-showcase/test/no-startup-warnings.test.ts). Known cost, put to the maintainer in the round report: the CLI's boot window drops in-window info (BOOT_DIAGNOSTIC_FLOOR = 'warn'), so an os dev boot does not show the line. It reaches hosts logging at info outside that window. Moving to warn is a one-word change if the maintainer weighs visibility higher.
  • Ablation: 3 legs, each declared first, landed on disk and restored to the HEAD blob. (1) Without the mask branch: 10 failed. (2) Value-only check: 1 failed, the literal-bullets pin. (3) Declaration-only check: 2 failed, the presence pin on both paths. Each check carries its own pin.
  • Real engine: the dev's uncommitted harness ran all 132 showcase rows on one SQLite file. Boot 2 updated 0 rows and printed 1 notice naming showcase_field_zoo.f_password. With the mask branch ablated, "Specimen — Full" was rewritten again.
  • Landing: not armed yet. CI on this head is converging: 12 of 37 in progress, none red, 6 skips all on the expected roster. This PR also sits behind the same main hold as fix(objectql,lint)!: a conditional rule's when reads one hop through a reference, hydrated like a condition #24650, fix(objectql): a runAs:'user' hook carries its user's email, so a current_user.email row policy admits it as it admits the user's own request #24674 and docs(metadata-core): name the real write-path labels on MetadataEvent.source #24675 (seat post [PM seat] domain:engine · seat 2 — ⏳ vacant #20966): main's Dogfood red is attributed to feat(spec/ui): export navItemVisibleDataNeedsDataSource, so a mirror of NavigationItemSchema attaches it by identifier #24628 (anchor dogfood: main's Dogfood Regression Gate (3/3) is red since #24628 — the ADR-0058 D7 expression-surface ledger reads nine unclassified ui/app.zod.ts positions and a three-way ratchet-key collision #24689), and a single Temporal Conformance timeout in this lane waits on the 00:01Z full battery.

Generated by Claude Code

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/m tests tooling

Projects

None yet

2 participants