Skip to content

Commit a7de844

Browse files
fix(objectql): deleting a record releases the sys_secret row its secret field named (#27657)
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F <!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuPMZ8BHWtPf6nBTp51qSM64)_ Fixes #27581 Clause-②: no Metadata-protocol: none Skills-docs: none — nothing a metadata author writes changes; the fix is inside the engine's delete path, and the documented `os secret orphans` report is unchanged. ## 维护者速读 - 改了什么:删除记录后,释放该记录 `secret` 字段引用的 `sys_secret` 凭据行。覆盖按 id 删除、按条件批量删除、REST 的 `deleteMany`,以及级联删除连带删掉的子记录。 - 为什么改:删除流程原先不读被删行的 `secret` 列,凭据密文留在库里,没有任何记录引用它,却仍可解密;凭据表随每次删除增长。 - 风险与回滚:沿用 #27572 已上线的同一条释放规则:同一列没有其他行还引用该句柄,句柄属于这一列,删除跟随记录所在的事务;核实不了就保留该行并记一条 error 日志,这是可恢复的方向。被拒绝或回滚的删除不释放。回滚只需 revert 本 PR。 - 先看:`packages/objectql/src/engine.ts`(delete 流程里取句柄的三处,以及 `afterDelete` 之后的释放点),`packages/objectql/src/engine-secret-internal-fields.ts`(`deletedSecretSlots`),`packages/objectql/src/secret-field-write-commit.test.ts`(新增的 delete 一段)。 - 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run src/secret-field-write-commit.test.ts`,预期 30/30 通过,其中 9 条是本 PR 新增。 - 需要你做的:无,席位落地。 ## What changes A committed delete now releases every handle the deleted rows held in a `secret` column, through the release #27572 added for a committed update that clears the field. There is one release rule; this change only hands it the delete's candidates. - **Candidates, from reads the delete already makes.** `deletedSecretSlots` (in `engine-secret-internal-fields.ts`) lists each handle a deleted row held in a `secret` column. It is `retiredSecretSlots` called with a `null` clear of every secret column, so a delete's candidates are exactly what clearing those columns would retire. - By id: the pre-image the delete already reads for its 404 gate. - By predicate, no cascade plan: the per-row hook read when one was made; otherwise one narrow `id` + secret-columns read through the write's AST and driver options, before the statement. An object with no `secret` field reads nothing new. - By predicate, with a cascade plan: the matched-row read inside the unit now also carries the secret columns, so the handles released are those of the rows deleted. - A cascade's detail records are deleted through the by-id path, so each releases its own handles. - **Release, verified, after the hooks.** `releaseSecretHandles(object, deletedSecrets, context, 'retired')` runs after the `afterDelete` chain and its result-shape check, before the roll-up recompute. A hook can still dereference the value, and a delete that is refused or rolled back never reaches it. Each candidate goes only when no remaining row of its column names the handle, the `sys_secret` row's `namespace` / `key` are that column's own, and the count and delete ride the record's unit. Otherwise the row is kept and one `error` line names the column. - **Not in this change.** Legacy orphans are not swept; they stay report-only through `os secret orphans`, per #8259. What a delete answers and what reads return are unchanged. ## Tests - `packages/objectql/src/secret-field-write-commit.test.ts`, a new `[#27581]` block of 9 cases on the existing double. - Released: a by-id delete (and no other record's row); a predicate delete (the narrow read); a predicate delete with a per-row hook (the hook's read); a cascade, by id and by predicate, releasing the detail rows' handles with the parent's. - Kept: a handle another row still names, until the last row goes; a row holding another column's handle; a delete rolled back with its unit; a delete refused by `beforeDelete`, and one rolled back by an aborting `afterDelete`; a release that cannot confirm, which logs one line naming the column and not the handle. - The double gained `deleteMany` and an isolation mode where a rollback undoes only the writes made under that unit's handle, as a database does. In the default snapshot mode a rollback restores every store, so a release that escaped the unit could not be seen. - `packages/qa/dogfood/test/secret-field-delete-release.dogfood.test.ts` (new, 4 cases), at the REST data door of a booted stack with a provider wired, counting `sys_secret` rows at the driver. - `DELETE /data/OBJECT/ID` removes that record's row and no other. - `POST /data/OBJECT/deleteMany` removes the row of every record it deletes. - A handle another record still names is kept until the last one is deleted. - An atomic `deleteMany` that rolls back (its second id names no record) keeps the record and its row. ## Acceptance notes - **Which delete paths exist.** By id; by predicate (`multi: true`) on three plans (no relation points at the object; a cascade inside one unit; a split cascade); cascade members, deleted by id. The REST `DELETE`, the bulk `deleteMany` door (per-id engine deletes, atomic or not) and the cross-object `/batch` delete all reach the engine's delete by id. **There is no soft-delete or recycle-bin path**: `softDelete` and `enable.trash` were retired from the spec in the 16.x line as never enforced, and no runtime keeps a deleted record, so there is no "kept record" case to protect. - **One release rule.** The release body is unchanged. The delete passes the existing `'retired'` occasion, so a kept row logs the same line ("the sys_secret row that OBJECT.FIELD named before this write was not removed"). No second rule and no new log text. - **The rollback pin at the REST door, and its ablation.** On the dogfood stack's `sqlite-wasm` driver, the release that does not ride the unit goes red **by timeout** (`Test timed out in 5000ms`), not by an assertion. The batch holds the pool's single connection, so a statement outside the unit waits for it. The assertion-level red for "the release rides the unit" is the engine case on the double's isolation mode (ablation A3 below). - **Scope in `engine.ts`.** The delete flow, two names in the secret-fields import list, and one sentence in `releaseSecretHandles`'s docblock naming the delete occasion. `engine-secret-internal-fields.ts` gains the two small helpers and one header sentence. - This branch is the card thread's harness-pinned `claude/project-thread-v9mp90`; the claim on #27581 (6500734029) names it. ## Verification Final head `b15352e9` is `54efb0b8` with `origin/main` (`f7ff693c`) merged in. The red-first and ablation legs ran at `54efb0b8`, with the fix committed. The final-head readings below were taken at `b15352e9` after rebuilding what the merge touched. <details><summary>Red first, unit, typecheck, lint and the REST door</summary> - Red first, on the unfixed tree (`engine.ts` and `engine-secret-internal-fields.ts` at base `9c513147`, the new tests kept): `secret-field-write-commit.test.ts` showed 6 failed / 24 passed (30). The 6 were the four released cases, the shared-handle case's last delete, and the cannot-confirm case. The 3 kept cases were green there, as a keep should be. - `pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=3` at `b15352e9`: Test Files 488 passed (488), Tests 9163 passed (9163). - `pnpm --filter @objectstack/objectql typecheck`: exit 0; `check:test-typecheck: OK`, 39 files / 233 errors / 64 pinned signatures held, unchanged. - `pnpm --filter @objectstack/dogfood typecheck`: exit 0. - `pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2 test/secret-field-delete-release.dogfood.test.ts test/secret-field-orphan-rows.dogfood.test.ts` at `b15352e9`: Test Files 2 passed (2), Tests 9 passed (9). - `eslint` on the four changed source and test files: exit 0. `pnpm lint` (the repo-wide run) at `b15352e9`: exit 0. - The full dogfood battery is not derived here: the title has no `!` and `Clause-②` has no `(narrowing)` arm. </details> <details><summary>Ablation (each through scripts/ablation-replace.mjs, restore proven)</summary> Source-resolved, against `secret-field-write-commit.test.ts`. Every mutation landed (anchor 1 → 0), and every restore came back to the HEAD blob (`engine.ts` `ab2b884a7373`, `engine-secret-internal-fields.ts` `621c5ec3c95f`) with `git diff HEAD` empty. - A1, the delete's release call removed: 6 failed / 24 passed, the same six as the unfixed tree. - A2, the shared-handle check removed: 2 failed (the #27495 shared-handle case and this PR's delete one). - A3, the release's store options stripped of the unit: 1 failed (the delete rolled back with its unit, on the isolating double). - A4, the column-ownership check removed: 2 failed (the #27495 foreign-column case and this PR's delete one). Dist-resolved, which the dogfood file consumes. Each mutation carried a marker; `@objectstack/objectql` was rebuilt and `ablation-dist-preflight` found the marker in 4 built files before the run. - D1, the delete's release call removed: 3 failed / 1 passed (DELETE by id, the `deleteMany` door, the shared handle's last delete). - D2, the shared-handle check removed: 1 failed (the shared handle, gone after the first delete). - D3, the release's store options stripped of the unit: 1 failed, the atomic rollback, by test timeout (see the acceptance notes). - Restore: rebuilt with declarations, `--absent` exit 0 for all three markers, the file 4 passed (4), and the tree clean against HEAD. </details> <details><summary>Local gate subset</summary> - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived 67 commands at `b15352e9`. 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. - `check:dual-build-cjs-loads` answered exit 3 on its first run (seven packages had no `dist/`); those were built, and it is exit 0 above. </details> <details><summary>Overlap with open PRs</summary> `git merge-tree` of this head against every open PR whose diff touches these files: #27477 (head `5eb62f09`, on `engine.ts`) merges clean. No other open PR touches them, measured over every open PR. The server seat 1 hot-file queue names #25522 and then #27165 for `engine.ts`; neither has an open PR. This head also merges clean with the current `origin/main`. </details> The required CI families run on this PR's check-runs and are not re-run here. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (Not touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgUFEohazYphk3zDAum)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgUFEohazYphk3zDAum --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3b0b577 commit a7de844

5 files changed

Lines changed: 384 additions & 12 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): deleting a record releases the `sys_secret` row its `secret` field named (#27581)
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **Released when the record goes.** A committed delete now removes the `sys_secret` row each deleted record's `secret` field named: a delete by id, a predicate delete (`multi: true`, and so the bulk `deleteMany` door, which deletes by id), and every detail record a cascade deletes, since those go through the by-id delete. The handles come from the rows the delete already reads before its statement (the by-id pre-image, the predicate path's matched rows), plus one narrow `id` + secret-columns read on a predicate delete that read no rows otherwise; an object with no `secret` field reads nothing new.
11+
- **One release rule.** The removal goes through the same verified release a committed update uses when it clears a `secret` field: no remaining row of that column may still name the handle (a predicate update shares one handle across every row it matched), the handle must be that column's own, and the removal rides the record's transaction where one is open. It runs after the `afterDelete` hooks, so a hook can still read the value and a delete that is refused or rolled back keeps the row. A removal that cannot be confirmed keeps the row and logs one `error` line naming the column; `os secret orphans` reports such rows.
12+
- **Unchanged.** What a delete answers, and what reads return. There is no soft-delete or recycle-bin path to cover. Credential rows left behind before this release are not swept; they stay report-only through `os secret orphans`.

‎packages/objectql/src/engine-secret-internal-fields.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
* bodies joined it — `persistSecretRows` writes the sealed rows immediately
2121
* before the record's own driver write, and `releaseSecretHandles` removes a
2222
* handle once a write that did not land, or a committed one that moved the
23-
* column off it, has left it unreferenced.
23+
* column off it, has left it unreferenced. [#27581] A committed delete hands
24+
* the same release every handle the deleted rows held (`deletedSecretSlots`).
2425
*
2526
* ## How the moved bodies reach the class
2627
*
@@ -228,6 +229,30 @@ export function retiredSecretSlots(
228229
return out;
229230
}
230231

232+
/**
233+
* [#27581] The `secret` columns a delete reads off the rows it removes, before
234+
* its statement takes them away; empty for an object that declares none, so a
235+
* delete anywhere else reads nothing more.
236+
*/
237+
export function deletedSecretColumns(schema: Parameters<typeof collectSecretFields>[0]): string[] {
238+
return collectSecretFields(schema);
239+
}
240+
241+
/**
242+
* [#27581] The handles a committed delete let go of: every handle the deleted
243+
* rows held in a `secret` column. A delete lets go of each column's value at
244+
* once, so the candidates are exactly what a `null` clear of every one of
245+
* those columns retires ({@link retiredSecretSlots}), and the same
246+
* `releaseSecretHandles` checks decide each one: there is one release rule.
247+
*/
248+
export function deletedSecretSlots(
249+
schema: Parameters<typeof collectSecretFields>[0],
250+
deletedRows: readonly (Record<string, unknown> | null | undefined)[],
251+
): SecretHandleSlot[] {
252+
const clearEvery = Object.fromEntries(deletedSecretColumns(schema).map((field) => [field, null]));
253+
return retiredSecretSlots(schema, deletedRows, clearEvery);
254+
}
255+
231256
/** The body of `ObjectQL#persistSecretRows`; the method's docblock there is its contract. */
232257
export async function persistSecretRows(
233258
this: Pick<SecretInternalFieldsHost, 'getDriver'>,

‎packages/objectql/src/engine.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ import { publishBulkDataEvent, publishDataEvent } from './engine-realtime-publis
108108
// [#25522 cut 5] The secret and internal-field bodies; the methods of the same
109109
// names below hand each body the members it reads (`SecretInternalFieldsHost`).
110110
import {
111+
deletedSecretColumns,
112+
deletedSecretSlots,
111113
encryptSecretFields,
112114
maskSecretFields,
113115
omitInternalFields,
@@ -10618,7 +10620,9 @@ export class ObjectQL implements IObjectQLEngine {
1061810620
* Two occasions reach it. `'refused'`: a write that sealed and wrote a row
1061910621
* did not complete, so the row may name nothing. `'retired'`: a committed
1062010622
* update cleared the field (`null`) or replaced its value, so the handle the
10621-
* column named before may name nothing. The same three checks decide both,
10623+
* column named before may name nothing; [#27581] a committed delete reaches
10624+
* it the same way, for every handle the removed rows held. The same three
10625+
* checks decide both,
1062210626
* and every one that cannot pass KEEPS the row, the recoverable direction
1062310627
* (deleting a handle a row still names destroys that credential, and nothing
1062410628
* can name what was lost):
@@ -19831,6 +19835,11 @@ export class ObjectQL implements IObjectQLEngine {
1983119835
// delete (`null` when no roll-up aggregates this object).
1983219836
let predicateMatchedRows: Record<string, unknown>[] | null = null;
1983319837
let summaryPriorRows: Record<string, unknown>[] | null = null;
19838+
// [#27581] The `secret` columns this object declares, and the handles the
19839+
// removed rows held in them: the by-id pre-image, or the predicate
19840+
// path's matched rows, read before the statement takes them away.
19841+
const secretColumns = deletedSecretColumns(deleteSchema);
19842+
let deletedSecrets: SecretHandleSlot[] = [];
1983419843

1983519844
// [#5574] The dispatch ladder, resolved BEFORE the before phase — see
1983619845
// update()'s twin for the full reasoning. Keyed on the SAME falsy-`id`
@@ -20052,6 +20061,8 @@ export class ObjectQL implements IObjectQLEngine {
2005220061
if (plan === 'split') this.warnCascadeNotAtomic(object);
2005320062
result = await runByIdDelete(opCtx.context);
2005420063
}
20064+
// [#27581] The handles the removed row held, off its pre-image.
20065+
deletedSecrets = deletedSecretSlots(deleteSchema, [priorRecord]);
2005520066
} else {
2005620067
// [#2982] The AST asserted present and already used for the
2005720068
// pre-phase row read above.
@@ -20086,6 +20097,14 @@ export class ObjectQL implements IObjectQLEngine {
2008620097
summaryPriorRows = predicateMatchedRows
2008720098
?? (((await driver.find(object, { ...opCtx.ast!, fields: summaryKeyColumns }, hookContext.input.options as Parameters<IDataDriver['find']>[2])) ?? []) as Record<string, unknown>[]);
2008820099
}
20100+
// [#27581] The handles the matched rows hold, read BEFORE the
20101+
// statement removes them: the per-row hook read when it was
20102+
// made, otherwise only `id` and the secret columns, through
20103+
// this write's AST and options. No secret column, no read.
20104+
if (secretColumns.length > 0) {
20105+
deletedSecrets = deletedSecretSlots(deleteSchema, predicateMatchedRows
20106+
?? (((await driver.find(object, { ...opCtx.ast!, fields: ['id', ...secretColumns] }, hookContext.input.options as Parameters<IDataDriver['find']>[2])) ?? []) as Record<string, unknown>[]));
20107+
}
2008920108
result = await driver.deleteMany!(object, opCtx.ast!, hookContext.input.options as any);
2009020109
} else {
2009120110
// Otherwise: resolve the matched rows through the composed,
@@ -20101,9 +20120,12 @@ export class ObjectQL implements IObjectQLEngine {
2010120120
// branch's, for the same reasons (#7413).
2010220121
const runPredicateDelete = async (writeContext?: ExecutionContext): Promise<unknown> => {
2010320122
const writeOptions = this.buildDriverOptions(object, writeContext, hookContext.input.options as any);
20104-
const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id', ...summaryKeyColumns] }, writeOptions)) ?? []) as Array<Record<string, unknown>>;
20123+
// [#27581] The same read carries the secret columns, so the
20124+
// handles released are those of the rows deleted.
20125+
const matched = ((await driver.find(object, { ...opCtx.ast!, fields: ['id', ...summaryKeyColumns, ...secretColumns] }, writeOptions)) ?? []) as Array<Record<string, unknown>>;
2010520126
const matchedIds = matched.map((row) => row?.id).filter((v): v is string | number => v != null);
2010620127
if (summaryKeyColumns.length > 0) summaryPriorRows = withoutDeletedSelfParents(object, matched, matchedIds, this.getSummaryDescriptors(object));
20128+
deletedSecrets = deletedSecretSlots(deleteSchema, matched);
2010720129
// [#27004] One walk, rooted at the whole matched set.
2010820130
await this.cascadeDeleteRelations(object, matchedIds, writeContext);
2010920131
// The id set rides an `$in`; batched so one statement never
@@ -20166,6 +20188,14 @@ export class ObjectQL implements IObjectQLEngine {
2016620188
});
2016720189
}
2016820190

20191+
// [#27581] The delete is complete: release the handles the removed
20192+
// rows held, through the release a committed update uses, so each one
20193+
// goes only once storage confirms no row of its column still names it,
20194+
// inside this write's unit. After the `afterDelete` chain, so a hook
20195+
// can still dereference the value and a refusal that rolls the delete
20196+
// back never reaches here.
20197+
await this.releaseSecretHandles(object, deletedSecrets, opCtx.context, 'retired');
20198+
2016920199
// Roll-up: recompute the parent summary now that the child is gone,
2017020200
// from the row's FK values captured BEFORE deletion. [#5272] That
2017120201
// capture is now the same single pre-image read `previous` rides on

0 commit comments

Comments
 (0)