Skip to content

fix(objectql): a secret field's sys_secret row is written only with its record, and released when the column lets go - #27572

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-r4cp59
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-r4cp59

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Sales · project thread

Fixes #27495

Clause-②: no
Metadata-protocol: none
Skills-docs: none — nothing a metadata author writes changes; the fix is inside the engine's credential write path, and the documented os secret orphans report is unchanged.

维护者速读

  • 改了什么:secret 字段的 sys_secret 凭据行改为只随记录本身的写入一起落库;写入没完成时收回它已写的凭据行;更新把字段清为 null 或换成新值后,释放旧值的凭据行。
  • 为什么改:凭据行原先在校验之前就写入,写入被拒也留下无人引用的密文;清空字段也不释放旧行。用户以为没存或已删的密文其实还留在库里。
  • 风险与回滚:释放前先核实三件事:同一列没有任何行还引用这个句柄,句柄是这一列自己的,删除跟随记录所在的事务。核实不了就保留该行并记一条 error 日志,这是可恢复的方向。读出的打码、resolveSecretField、调用方收到的拒绝都不变;历史遗留的孤儿行不清理,仍由 os secret orphans 报告。回滚只需 revert 本 PR。
  • 先看:packages/objectql/src/engine-secret-internal-fields.ts(风险在 releaseSecretHandles 的核实与事务判断),packages/objectql/src/engine.ts(insert / update 的写入点与两处释放点),packages/objectql/src/secret-field-write-commit.test.ts。
  • 两分钟验证:pnpm --filter @objectstack/objectql exec vitest run src/secret-field-write-commit.test.ts,预期 21/21 通过。
  • 需要你做的:无,席位落地。

What changes

The credential pass for a secret field is split into three steps on the existing secret-fields path (engine-secret-internal-fields.ts, whose bodies engine.ts calls).

  • Seal, before validation, as before. encryptSecretFields still encrypts the value and puts the secret: ref on the row, so validation and the post-image permission check judge the stored reference exactly as they did. It no longer writes the sys_secret row; it hands the sealed row back. The fail-closed refusals (no crypto provider, no sys_secret store, the empty string) are unchanged and still fire at this point.
  • Persist, with the record. persistSecretRows writes the sealed rows immediately before the record's own driver statement, after every pass that can refuse the write: on insert, only for the rows insertMany keeps; on update, on both the by-id and the predicate branch.
  • Release, verified. releaseSecretHandles runs on two occasions.
    • refused: a write that did not complete (validation, an option outside the list, a required field, a hook, the permission check, the driver refusing the statement) takes back the rows it wrote.
    • retired: a committed update that clears a secret field with null, or replaces its value, releases the handle the field named before. The candidates come from the by-id pre-image, or from the predicate path's prior rows (the D7 memo when it was read, else one narrow id + secret-columns read). An echoed ref or an echoed mask releases nothing. It runs after the afterUpdate chain, so a refusal that rolls the record back never reaches it.
    • Each candidate is checked against storage before anything is deleted. No row of that column may still name the handle (a predicate update shares one handle across every row it matched). The sys_secret row's namespace / key must be this object and field. The delete rides the record's transaction: the explicit, still-open handle, else the ambient unit. If the record write rode a unit that does not cover sys_secret, the row is kept, because the record could still roll back and name it.
    • Any failure keeps the row and logs one error line that names the column, never the handle id, with the driver fault redacted, and points at os secret orphans. The caller's answer never changes.
  • Not in this change. Legacy orphans are not swept; they stay report-only, per [Decision] sys_secret orphan reaping: which vehicle carries a destructive sweep over stored credentials — migration, admin command, or opt-in script? #8259. Reads, resolveSecret / resolveSecretField, the mask round trip and every refusal envelope are unchanged. Deleting a record is not touched.

Tests

  • packages/objectql/src/secret-field-write-commit.test.ts (new, 21 cases): a real engine over a recording driver with snapshot transactions.
    • A refused write leaves the sys_secret count unchanged: insert refused by a required field, an option outside the list, a beforeInsert hook, the post-image permission check, the driver refusing the statement, a batch statement, and a partial insertMany; update refused by an option outside the list, the permission check, the driver; a predicate update refused by an option outside the list and by the driver; an afterInsert abort.
    • A committed write still stores, masks and dereferences; an echoed mask releases nothing.
    • A null clear removes the old row; a replacement removes the old row and keeps the new one; a predicate clear removes every matched row's handle; a handle a predicate write shared outlives every row but the last; a handle minted for another column is never released through this one; a release that cannot confirm keeps the row and logs one line naming the column and not the handle.
  • packages/qa/dogfood/test/secret-field-orphan-rows.dogfood.test.ts (new, 5 cases): the same halves at the REST data door of a booted stack with a provider wired, counting sys_secret rows at the driver.

Acceptance notes

  • The card asks for every refusal class. Three of them (a before* hook, the insert-side permission check, an after* abort) already wrote no row before this change, because they refuse before the credential pass or roll back with the record. Their cases were green on the unfixed tree and hold that order; the other classes were red there (15 of 21).
  • Replacing a value is the same mechanism as clearing it (the column lets go of the old handle), so both are released.
  • The claim's file surface names secret-fields.ts and the engine's write and clear steps. Since the engine split, those steps' bodies live in engine-secret-internal-fields.ts; secret-fields.ts itself needed no change.
  • This branch is the card thread's harness-pinned claude/project-thread-r4cp59; the claim on objectql: a refused write carrying a secret field still mints an orphan sys_secret row, and clearing a secret with null leaves its sys_secret row behind #27495 names it.

Verification

All readings below were taken at head c0b6bfc7, after origin/main (d4ff1a02) was merged in and the dogfood dependency closure was rebuilt, unless a line says otherwise.

Unit, typecheck, lint and the REST door
  • Red first, on the unfixed tree: secret-field-write-commit.test.ts showed 15 failed / 6 passed (21), each failure on a sys_secret count.
  • pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=3: Test Files 487 passed (487), Tests 9143 passed (9143).
  • pnpm --filter @objectstack/objectql typecheck: exit 0; check:test-typecheck: OK, 39 files / 233 errors / 64 pinned signatures held, unchanged.
  • eslint on the three changed packages/objectql files: exit 0.
  • pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2 test/secret-field-orphan-rows.dogfood.test.ts: Tests 5 passed (5).
  • pnpm --filter @objectstack/dogfood typecheck: exit 0. eslint on the dogfood file: exit 0.
  • The full dogfood battery is not derived here: the title has no ! and Clause-② has no (narrowing) arm.
Ablation (each through scripts/ablation-replace.mjs, restore proven)

Source-resolved, against secret-field-write-commit.test.ts; every mutation landed (anchor 1 → 0) and every restore came back to the HEAD blob 163a38d28da2 with git diff HEAD empty.

  • The seal also writes the row (the old timing): 10 failed / 11 passed. Every refused-write case the fix moved went red.
  • The release's delete made a no-op: 8 failed / 13 passed (the clear, replace, predicate-clear and shared-handle cases, and the four driver-refusal take-backs).
  • The shared-handle check removed: 1 failed (the shared-handle case).
  • The column-ownership check removed: 1 failed (the foreign-column case).

Dist-resolved, which the dogfood file consumes (taken at 33868e7d; the one later commit touches only the unit test's double): each mutation carried a marker, @objectstack/objectql was rebuilt and ablation-dist-preflight found the marker in dist/.

  • The old write timing: 2 failed / 3 passed (both refused-write cases).
  • The release's delete made a no-op: 2 failed / 3 passed (the clear and replace cases).
  • Restore: rebuilt, --absent clean for both markers, the file 5 passed (5), and the tree clean against HEAD.
Local gate subset
  • node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 67 commands at c0b6bfc7. All 67 were run, each exit code captured before any pipe: 67 × exit 0. --ran reconciliation: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.
  • On the first pass, at 33868e7d, two gates did not pass. check:objectql-double-limit flagged that the new test's recording double ignored the caller's limit; it now applies the bound after its filter (c0b6bfc7). check:dual-build-cjs-loads answered exit 3 (seven packages had no dist/); those were built, and both are exit 0 above.
Overlap with open PRs

git merge-tree of this head against every open PR whose diff touches these files: #27477, #27541 and #27543 (all on engine.ts) merge clean. No other open PR touches them. This head also merges clean with the current origin/main.

The required CI families run on this PR's check-runs and are not re-run here.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (Not touched.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes on the changed files.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the 维护者速读 block is the dispatch's required Chinese summary).
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01L6bhgXcN9HaZYSEUdJWEjw

…cord, and release it when the column lets go

The credential pass now seals a secret value (the ref still lands on the row
before validation and the post-image seam judge it) and hands the sys_secret
row back instead of writing it. The row is written immediately before the
record's own driver write, after every pass that can refuse the write, and
taken back when the write does not complete.

A committed update that clears a secret field or replaces its value releases
the handle the column named before. Every release is verified against
storage first: no row of the column may still name the handle (a predicate
write shares one handle across its rows), the handle must be that column's
own, and the delete rides the record's unit. Anything that cannot be
confirmed keeps the row and is reported at error.

Claude-Session: https://claude.ai/code/session_01L6bhgVuA8QG6cxFjdHMHgn
Co-authored-by: Claude <noreply@anthropic.com>
…a door; add the changeset

Claude-Session: https://claude.ai/code/session_01L6bhgVuA8QG6cxFjdHMHgn
Co-authored-by: Claude <noreply@anthropic.com>
… is kept; point at os secret orphans

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6bhgVuA8QG6cxFjdHMHgn
…after its filter

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6bhgVuA8QG6cxFjdHMHgn
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 15 documentable anchor(s).

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

  • content/docs/automation/webhooks.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/data-modeling/drivers.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/data-modeling/external-datasources.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/data-modeling/objects.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/data-modeling/validation-rules.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/deployment/backup-restore.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/deployment/cli.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/deployment/environment-variables.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/kernel/cluster.mdx (via driverOptions (symbol, a field of interface PendingSecretRow))
  • content/docs/permissions/authorization.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/protocol/kernel/config-resolution.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))

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

  • content/docs/releases/implementation-status.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/releases/v16.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/releases/v17/17-0.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/releases/v17/17-7.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))
  • content/docs/releases/v18/18-0.mdx (via sys_secret (literal, a string literal in encryptSecretFields; a string literal in persistSecretRows; a string literal in releaseSecretHandles))

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
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 78 pages)
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 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 — 19 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 556f3230cdd43caa6a263a35c2133e1e9fbfbefc → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 556f3230cdd43caa6a263a35c2133e1e9fbfbefc

⚠️ 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 556f3230cdd43caa6a263a35c2133e1e9fbfbefc → 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

domain:server#1 · os-sales · session_01L6bhgXcN9HaZYSEUdJWEjw

维护者速读(席位终稿)

  • 改了什么:secret 字段的 sys_secret 凭据行改为只随记录本身的写入一起落库;写入没完成时收回已写的凭据行;更新把字段清为 null 或换成新值后,释放旧值的凭据行。
  • 为什么改:objectql: a refused write carrying a secret field still mints an orphan sys_secret row, and clearing a secret with null leaves its sys_secret row behind #27495,凭据行原先在校验之前就写入,被拒的写入也留下无人引用的密文;清空字段也不释放旧行。用户以为没存或已删的密文其实还在库里。
  • 风险与回滚:中。释放前核实三件事(同列无行仍引用该句柄、句柄属于这一列、删除跟随记录所在事务),核实不了就保留并记一条 error,这是可恢复的方向。读出打码、resolveSecretField、拒绝报文都不变;遗留孤儿行不清理。回滚就是还原这一个 PR。
  • 先看:packages/objectql/src/engine-secret-internal-fields.ts 的 releaseSecretHandles,以及 engine.ts 中 insert / update 的写入点和两处释放点。
  • 两分钟验证:pnpm --filter @objectstack/objectql exec vitest run src/secret-field-write-commit.test.ts,预期 21/21 通过。
  • 需要你做的:无。Tier S,不碰契约四面,席位自读后落地。

席位意见:接受。拆成「封存 → 随记录写入 → 核实后释放」三步,失败一律保留凭据行,方向对;席位逐段读过 diff。

席位核对 — ACCEPT · domain:server#1 · head c0b6bfc · 2026-10-29T13:50Z

  • Shape: the credential pass is split in three on the existing secret-fields path. encryptSecretFields still seals before validation (fail-closed refusals unchanged), but now hands back PendingSecretRow[] instead of writing. persistSecretRows writes them immediately before the record's own driver statement: insert for live rows only, update on both the by-id and the predicate branch. releaseSecretHandles runs on two occasions. refused runs in the insert/update catch. retired runs after the afterUpdate chain, with candidates from the by-id pre-image or the predicate prior rows. Each candidate is checked against storage: a column count of 0, namespace / key equal to this object and field, and the delete riding the record's unit (kept when the unit does not cover sys_secret). Any failure keeps the row and logs one error naming the column, never the handle id.
  • Premise: claim 6495598668 holds; Fixes #27495; triage 6495345114. ⛔ Per the triage, no reproduction detail goes on the card or this PR. The body names the mechanism and the pins.
  • Gates: 49 check-runs on this head: 35 success, 14 skipped, 0 failed, 0 pending. check-expected-skips --pr 27572 (fresh origin/main): VERDICT … OK — 14 skipped check-run(s), every one in main's roster (exit 0). mergeable_state: clean. Dev (os-dev-report 6498080436): objectql 9143/9143, new unit file 21/21 (15 red on the unfixed tree), dogfood REST door 5/5, four unit ablation legs and two dist legs all red and restored to the HEAD blob, 67 derived gates exit 0, 0 NOT-MEASURED. Merge-tree is clean against fix(objectql): extend the credential-column guard from aggregates to filter and sort keys #27477, fix(objectql)!: refuse a projection whose every column is unknown, as an empty one is refused #27541, fix(i18n): a tenant-rewritten validation rule message beats the packaged translation, on the refusal and on /meta #27543 and main.
  • Contract review: not owed. Clause-②: no, no narrowing (no request is refused that was answered before; refusals and reads are unchanged), no packages/spec/**, no governed path. The seat read it per contract-review.md (四面皆不碰 ⇒ CI + seat read).
  • Seat read: engine-secret-internal-fields.ts and engine.ts in full. The count is unscoped by tenant, so a handle another tenant's row shares is never released. A ref copied onto another column fails the namespace / key check and is kept. An echoed ref or a dropped echoed mask is not a candidate. The retired release sits after afterUpdate, so a hook refusal never reaches it. Changeset: @objectstack/objectql: patch, no banner (not breaking).
  • Accepted deviations: a refused write whose statement aborted the transaction (Postgres) logs one Credential row kept line for a row the rollback itself removes. Noise only: the direction is kept, and no row is lost. The dist ablation script uses --hold / --restore without a trap, and restoration was proven. The edits sit in engine-secret-internal-fields.ts, where the split moved the bodies.
  • Declaration: Clause-②: no, Metadata-protocol: none, Skills-docs: none (the os secret orphans report is unchanged).
  • Out of scope: deleting a record does not release its secret field's credential row: objectql: deleting a record does not release the sys_secret row its secret field names #27581. A non-system write can store an existing secret: ref: objectql: a secret field accepts an existing secret: ref from a non-system write, so it can name another record's credential row #27582 (security; mechanism only). Legacy orphans stay report-only ([Decision] sys_secret orphan reaping: which vehicle carries a destructive sweep over stored credentials — migration, admin command, or opt-in script? #8259).
  • Landing: Tier S, NOT governed, 867 changed lines ≤ 3000 → pr_ready + automerge_enable via the relay now.

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

Development

Successfully merging this pull request may close these issues.

objectql: a refused write carrying a secret field still mints an orphan sys_secret row, and clearing a secret with null leaves its sys_secret row behind

1 participant