diff --git a/.changeset/27495-secret-field-orphan-rows.md b/.changeset/27495-secret-field-orphan-rows.md new file mode 100644 index 00000000000..5f59071cc12 --- /dev/null +++ b/.changeset/27495-secret-field-orphan-rows.md @@ -0,0 +1,12 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a `secret` field's `sys_secret` row is written only with the record that names it, and released when that record's column lets go of it (#27495) + +Clause-②: no +Metadata-protocol: none + +- **Written with the record.** The credential pass still seals the value before validation, so the row carries its `secret:` ref when validation and the post-image permission check judge it. The `sys_secret` row itself is now written immediately before the record's own driver write, after every pass that can refuse the write. A create or update refused for any reason (validation, an option outside the list, a required field, a `before*` hook, the permission check) writes no credential row, a row `insertMany` culls writes none, and a statement the driver refuses takes back the row it was given. +- **Released when the column lets go.** A committed update that clears a `secret` field with `null`, or replaces its value, removes the `sys_secret` row the field named before. The removal is verified first: no 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 delete rides the record's transaction where one is open. A removal that cannot be confirmed keeps the row and logs one `error` line naming the column; `os secret orphans` reports such rows. +- **Unchanged.** What a read returns (the mask), what `resolveSecret` / `resolveSecretField` return for a stored value, an echoed mask (still a no-op), and the refusal a caller receives. Credential rows left behind before this release are not swept; they stay report-only through `os secret orphans`. diff --git a/packages/objectql/src/engine-secret-internal-fields.ts b/packages/objectql/src/engine-secret-internal-fields.ts index 60feb52d476..163a38d28da 100644 --- a/packages/objectql/src/engine-secret-internal-fields.ts +++ b/packages/objectql/src/engine-secret-internal-fields.ts @@ -15,19 +15,26 @@ * the same ambient-transaction rule for the privileged reads. Each method's * docblock stays on the method in `engine.ts`, which is still the contract. * + * [#27495] One later change, on the write side: `encryptSecretFields` seals a + * value without writing its `sys_secret` row and hands the row back, and two + * bodies joined it — `persistSecretRows` writes the sealed rows immediately + * before the record's own driver write, and `releaseSecretHandles` removes a + * handle once a write that did not land, or a committed one that moved the + * column off it, has left it unreferenced. + * * ## How the moved bodies reach the class * * The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts` * header carries the full reasoning), applied per method as - * `engine-realtime-publish.ts` applies it. The class keeps all nine methods — + * `engine-realtime-publish.ts` applies it. The class keeps all eleven methods — * the published `.d.ts` lists each one's name and position — and each body is * one literal of exactly the {@link SecretInternalFieldsHost} members that - * method reads, plus a `.call`. The three fields the bodies read - * (`_registry`, `cryptoProvider`, `txStore`) go as GETTERS, so each read - * happens when the body makes it: `setCryptoProvider` replaces the provider at - * any time, and a body sees whatever is installed at the moment it reads, as - * it did. The five methods they call (`getDriver`, - * `transactionCoversDriverFor`, `omitInternalFields`, + * method reads, plus a `.call`. The four fields the bodies read + * (`_registry`, `cryptoProvider`, `txStore`, `logger`) go as GETTERS, so each + * read happens when the body makes it: `setCryptoProvider` replaces the + * provider at any time, and a body sees whatever is installed at the moment it + * reads, as it did. The six methods they call (`getDriver`, + * `transactionCoversDriverFor`, `isClosedUnitHandle`, `omitInternalFields`, * `privilegedReadDriverOptions`, `resolveSecret`) go as late-bound closures * over the instance, so a double or spy set on the engine — tests replace * `resolveInternalField` and `resolveSecretField` on the instance — is still @@ -40,8 +47,9 @@ */ import type { CryptoHandle } from '@objectstack/spec/contracts'; +import type { DriverOptions } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -import { redactPropagatedDriverFault } from '@objectstack/types'; +import { redactBoundStatement, redactPropagatedDriverFault } from '@objectstack/types'; import { collectSecretFields, collectMaskedReadFields, @@ -68,13 +76,35 @@ export interface SecretInternalFieldsHost { readonly _registry: ObjectQL['_registry']; readonly cryptoProvider: ObjectQL['cryptoProvider']; readonly txStore: ObjectQL['txStore']; + readonly logger: ObjectQL['logger']; getDriver: ObjectQL['getDriver']; transactionCoversDriverFor: ObjectQL['transactionCoversDriverFor']; + isClosedUnitHandle: ObjectQL['isClosedUnitHandle']; omitInternalFields: ObjectQL['omitInternalFields']; privilegedReadDriverOptions: ObjectQL['privilegedReadDriverOptions']; resolveSecret: ObjectQL['resolveSecret']; } +/** + * [#27495] A `sys_secret` row the credential pass sealed for one record write + * and has not written yet: the column it was minted for, the handle id the + * row's ref now names, the row itself, and the driver options the pass was + * handed. {@link persistSecretRows} writes it immediately before the record's + * own driver write, so a write refused before that point leaves nothing behind. + */ +export interface PendingSecretRow { + readonly field: string; + readonly handleId: string; + readonly row: Record; + readonly driverOptions: unknown; +} + +/** [#27495] A handle a write may have left unreferenced, and the column it was minted for. */ +export interface SecretHandleSlot { + readonly field: string; + readonly handleId: string; +} + /** The body of `ObjectQL#encryptSecretFields`; the method's docblock there is its contract. */ export async function encryptSecretFields( this: Pick, @@ -82,8 +112,9 @@ export async function encryptSecretFields( row: Record, context: ExecutionContext | undefined, driverOptions: unknown, -): Promise { - if (!row || typeof row !== 'object') return; +): Promise { + const pending: PendingSecretRow[] = []; + if (!row || typeof row !== 'object') return pending; const schema = this._registry.getObject(object); // Echoed-mask drop for every field the read path masks (secret + generic @@ -97,7 +128,7 @@ export async function encryptSecretFields( } const secretFields = collectSecretFields(schema); - if (secretFields.length === 0) return; + if (secretFields.length === 0) return pending; for (const field of secretFields) { if (!(field in row)) continue; @@ -129,9 +160,11 @@ export async function encryptSecretFields( tenantId: context?.tenantId, }); - let secretDriver; + // Fail-closed on a missing store HERE, before validation, as before: the + // row is only written later (#27495), but whether it could be is a verdict + // on the composition, not on the record. try { - secretDriver = this.getDriver('sys_secret'); + this.getDriver('sys_secret'); } catch { throw new Error( `Cannot persist secret field "${object}.${field}": the sys_secret store is not available. ` @@ -139,9 +172,13 @@ export async function encryptSecretFields( ); } - await secretDriver.create( - 'sys_secret', - { + // [#27495] Sealed, not written: the ref goes on the row now, so validation + // and the post-image seam judge the stored reference as they always have, + // and the row reaches `sys_secret` only with the record that names it. + pending.push({ + field, + handleId: handle.id, + row: { id: handle.id, namespace: object, key: field, @@ -151,11 +188,124 @@ export async function encryptSecretFields( ciphertext: handle.ciphertext, created_at: new Date().toISOString(), }, - driverOptions as any, - ); + driverOptions, + }); row[field] = makeSecretRef(handle.id); } + return pending; +} + +/** [#27495] The `secret` fields a write payload carries a key for — the columns the write can move. */ +export function secretFieldsWritten( + schema: Parameters[0], + payload: unknown, +): string[] { + if (!payload || typeof payload !== 'object') return []; + return collectSecretFields(schema).filter((field) => field in (payload as Record)); +} + +/** + * [#27495] The handles a committed update moved its columns off: for each + * `secret` field the payload carries, every prior row's handle the payload + * does not keep — a `null` clear or a new value, never an echoed ref or a + * dropped echoed mask. A candidate list, not a verdict: `releaseSecretHandles` + * verifies each one against storage before anything is removed. + */ +export function retiredSecretSlots( + schema: Parameters[0], + priorRows: readonly (Record | null | undefined)[], + payload: unknown, +): SecretHandleSlot[] { + const out: SecretHandleSlot[] = []; + for (const field of secretFieldsWritten(schema, payload)) { + const next = (payload as Record)[field]; + for (const prior of priorRows) { + const handleId = parseSecretRef(prior?.[field]); + if (handleId && next !== makeSecretRef(handleId)) out.push({ field, handleId }); + } + } + return out; +} + +/** The body of `ObjectQL#persistSecretRows`; the method's docblock there is its contract. */ +export async function persistSecretRows( + this: Pick, + pending: readonly PendingSecretRow[], + written: PendingSecretRow[], +): Promise { + for (const one of pending) { + await this.getDriver('sys_secret').create('sys_secret', one.row, one.driverOptions as any); + written.push(one); + } +} + +/** The body of `ObjectQL#releaseSecretHandles`; the method's docblock there is its contract. */ +export async function releaseSecretHandles( + this: Pick, + object: string, + slots: readonly SecretHandleSlot[], + context: ExecutionContext | undefined, + occasion: 'refused' | 'retired', +): Promise { + if (slots.length === 0) return; + const kept = (slot: SecretHandleSlot, why: string, error?: unknown): void => { + // The column, never the handle id: a handle is what a stored ref names. + const what = occasion === 'refused' + ? `the sys_secret row sealed for a write on "${object}.${slot.field}" that did not complete` + : `the sys_secret row that "${object}.${slot.field}" named before this write`; + const redacted = error === undefined ? undefined : redactBoundStatement(error); + this.logger.error( + `Credential row kept: ${what} was not removed, because ${why}. The write's own answer is unchanged, ` + + 'but that ciphertext is still stored and decryptable with nothing pointing at it. ' + + '`os secret orphans` reports it.', + redacted instanceof Error ? redacted : undefined, + { object, field: slot.field }, + ); + }; + // The unit the record write rode, chosen as `buildDriverOptions` chooses it: + // an explicit, still-open handle first, then the ambient one. + const explicit = context?.transaction; + const tx = explicit !== undefined && !this.isClosedUnitHandle(explicit) + ? explicit + : this.txStore.getStore()?.transaction; + const recordInUnit = tx !== undefined && this.transactionCoversDriverFor(object, tx); + const storeInUnit = tx !== undefined && this.transactionCoversDriverFor('sys_secret', tx); + const seen = new Set(); + for (const slot of slots) { + const dedupe = `${slot.field}:${slot.handleId}`; + if (seen.has(dedupe)) continue; + seen.add(dedupe); + // A record write inside a unit the store is not part of can still roll + // back after this delete, and would then name a row that is gone: keep + // the orphan, the recoverable direction. + if (recordInUnit && !storeInUnit) { + kept(slot, 'the record write ran in a transaction that does not cover the sys_secret store, so the delete could outlive a rollback'); + continue; + } + try { + // Verify, never infer: no row of the column may still name the handle — + // a predicate write shares one handle across every row it matched. + const recordOptions: DriverOptions | undefined = recordInUnit ? { transaction: tx } : undefined; + const refs = await this.getDriver(object).count( + object, + { where: { [slot.field]: makeSecretRef(slot.handleId) } }, + recordOptions, + ); + if (refs > 0) continue; + const storeOptions: DriverOptions | undefined = storeInUnit ? { transaction: tx } : undefined; + const secretDriver = this.getDriver('sys_secret'); + const found = await secretDriver.find('sys_secret', { where: { id: slot.handleId } }, storeOptions); + const secret: any = Array.isArray(found) ? found[0] : found; + if (!secret) continue; + // Only this column's own handle: a ref a system writer copied onto + // another column says nothing about the column the handle was minted for. + if (secret.namespace !== object || secret.key !== slot.field) continue; + await secretDriver.delete('sys_secret', slot.handleId, storeOptions); + } catch (e) { + kept(slot, 'storage could not confirm the column let go of it, or could not delete it', e); + } + } } /** The body of `ObjectQL#refuseEmptyPasswordFields`; the method's docblock there is its contract. */ diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 2b9a80b31b7..c56ca62938e 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -111,12 +111,18 @@ import { encryptSecretFields, maskSecretFields, omitInternalFields, + persistSecretRows, privilegedReadDriverOptions, refuseEmptyPasswordFields, + releaseSecretHandles, resolveInternalField, resolveSecret, resolveSecretField, + retiredSecretSlots, + secretFieldsWritten, stripSearchCompanionFromRead, + type PendingSecretRow, + type SecretHandleSlot, } from './engine-secret-internal-fields.js'; // [#25522 cut 6] The predicate-write hook dispatch bodies; the methods of the // same names below hand each body the members it reads (`HookDispatchHost`). @@ -10536,9 +10542,12 @@ export class ObjectQL implements IObjectQLEngine { * Two channels share the read mask ({@link SECRET_MASK}) but differ on write * (see ADR-0100): * - * - **`secret`** — encrypted: each plaintext is wrapped by the ICryptoProvider, - * persisted as a `sys_secret` row, and replaced on `row` by an opaque ref. - * Cleartext never reaches the business table. + * - **`secret`** — encrypted: each plaintext is wrapped by the ICryptoProvider + * and replaced on `row` by an opaque ref. Cleartext never reaches the + * business table. [#27495] The `sys_secret` row is SEALED here and handed + * back, not written: the caller writes it with {@link persistSecretRows} + * immediately before the record's own driver write, so a write refused + * after this pass (validation, the post-image seam) leaves no row behind. * - **`password`** (generic, non-`better-auth`) — plaintext at rest: stored * verbatim, no encryption and no `sys_secret` row. Only the echoed-mask drop * below applies to it. @@ -10569,7 +10578,7 @@ export class ObjectQL implements IObjectQLEngine { row: Record, context: ExecutionContext | undefined, driverOptions: unknown, - ): Promise { + ): Promise { // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const readCryptoProvider = () => this.cryptoProvider; @@ -10581,6 +10590,73 @@ export class ObjectQL implements IObjectQLEngine { return encryptSecretFields.call(host, object, row, context, driverOptions); } + /** + * [#27495] Write the `sys_secret` rows {@link encryptSecretFields} sealed for + * one record write. Called immediately before that record's driver write and + * after every pass that can refuse it, so the credential row is written only + * with the record that names it. Each row lands on `written` as it is + * written, which is what the write's failure path hands to + * {@link releaseSecretHandles} when the record write does not land. + */ + private async persistSecretRows(pending: readonly PendingSecretRow[], written: PendingSecretRow[]): Promise { + // [#27495] The body lives in `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + const host: ThisParameterType = { + getDriver: (...a) => this.getDriver(...a), + }; + return persistSecretRows.call(host, pending, written); + } + + /** + * [#27495] Remove each `sys_secret` row in `slots` that the column it was + * minted for no longer names — best-effort, and never a throw. + * + * Two occasions reach it. `'refused'`: a write that sealed and wrote a row + * did not complete, so the row may name nothing. `'retired'`: a committed + * update cleared the field (`null`) or replaced its value, so the handle the + * column named before may name nothing. The same three checks decide both, + * and every one that cannot pass KEEPS the row, the recoverable direction + * (deleting a handle a row still names destroys that credential, and nothing + * can name what was lost): + * + * - **Verified, never inferred.** No row of `object` may still name the + * handle in that column: a predicate write seals ONE handle for every row + * it matches, so clearing one of them leaves the handle in force for the + * rest; a write that did land after all (a partially applied batch) still + * names its row. + * - **This column's own handle.** The `sys_secret` row's `namespace`/`key` + * must be this object and field; a ref a system writer copied onto another + * column is not released through that column. + * - **Inside the record's unit.** The count and the delete ride the open + * transaction where it covers their drivers, so they see the write's own + * state and roll back with it; a record write in a unit that does not + * cover the `sys_secret` store keeps the row, since the delete could + * outlive a rollback of the record. + * + * A row kept for a failure (not for being in use) is reported once at + * `error`, naming the column and the remedy: the caller's answer does not + * change, and the stored ciphertext is exactly what the operator believes + * gone. Legacy orphans written before this existed are not swept here; the + * orphan report lists them. + */ + private async releaseSecretHandles( + object: string, + slots: readonly SecretHandleSlot[], + context: ExecutionContext | undefined, + occasion: 'refused' | 'retired', + ): Promise { + // [#27495] The body lives in `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`). + const readTxStore = () => this.txStore; + const readLogger = () => this.logger; + const host: ThisParameterType = { + get txStore() { return readTxStore(); }, + get logger() { return readLogger(); }, + getDriver: (...a) => this.getDriver(...a), + transactionCoversDriverFor: (...a) => this.transactionCoversDriverFor(...a), + isClosedUnitHandle: (...a) => this.isClosedUnitHandle(...a), + }; + return releaseSecretHandles.call(host, object, slots, context, occasion); + } + /** * [#8559] Refuse `""` on a `password`-typed field before it reaches the * driver — the `password` half of the same maintainer ruling (2026-08-13) @@ -14893,7 +14969,7 @@ export class ObjectQL implements IObjectQLEngine { // // #8682's rule applied one step in — a refusal must cost nothing — so // this runs before `resolveSystemInsertOrganization`, before the - // credential channel (`encryptSecretFields` writes a `sys_secret` row), + // credential channel (`encryptSecretFields` seals a `sys_secret` row), // before validation and before `applyAutonumbers`. Pre-statement, so the // driver is never reached: that is also what puts #8682's Half B // statement-and-values leak out of reach on this path, since both SQL @@ -14949,7 +15025,7 @@ export class ObjectQL implements IObjectQLEngine { // [#8682] Rows the declared-field door refused are already dead on // arrival — seeded before the strips, before the seam, and so before // the credential loop further down, which is the first pass with a - // real side effect (`encryptSecretFields` writes a `sys_secret` row): + // real side effect (`encryptSecretFields` seals a `sys_secret` row): // a culled row is neither stripped, nor judged, nor able to mint a // secret for a write that will never happen. const rowErrors: (unknown | undefined)[] = new Array(rows.length); @@ -15127,7 +15203,7 @@ export class ObjectQL implements IObjectQLEngine { // Placement obeys the rule #8682 wrote for the declared-field door and // commit b003cf2e8 restated for its post-hook half: a refusal must cost nothing. // This sits after that door and BEFORE every producer — - // `resolveSystemInsertOrganization`, `encryptSecretFields` (which writes + // `resolveSystemInsertOrganization`, `encryptSecretFields` (which seals // a `sys_secret` row), `applyAutonumbers` (which CONSUMES a sequence // number), validation and the statement. // @@ -15250,6 +15326,10 @@ export class ObjectQL implements IObjectQLEngine { for (const rowCtx of rowHookContexts) { rowCtx.input.options = driverOptions; } + // [#27495] The credential rows each row's secret fields are sealed into, + // and the ones this write has actually written to `sys_secret`. + const sealedSecrets: PendingSecretRow[][] = rows.map(() => []); + const writtenSecrets: PendingSecretRow[] = []; try { let result: any; @@ -15271,7 +15351,7 @@ export class ObjectQL implements IObjectQLEngine { // carries the secret-arm refusal). Same try, so partial mode // reports either refusal per-row instead of killing the batch. this.refuseEmptyPasswordFields(object, rows[i]); - await this.encryptSecretFields(object, rows[i], opCtx.context, driverOptions); + sealedSecrets[i] = await this.encryptSecretFields(object, rows[i], opCtx.context, driverOptions); } catch (e) { if (!partialMode) throw e; rowErrors[i] = e; @@ -15506,6 +15586,11 @@ export class ObjectQL implements IObjectQLEngine { const liveIndexes: number[] = []; for (let i = 0; i < rows.length; i++) if (rowErrors[i] === undefined) liveIndexes.push(i); const liveRows = liveIndexes.map((i) => rows[i]); + // [#27495] The live rows' credential rows, and only theirs, written now: + // every pass that can refuse a row has run, so a refused write or a + // culled row never reaches `sys_secret`. A statement that then fails + // takes them back in the `catch` below. + await this.persistSecretRows(liveIndexes.flatMap((i) => sealedSecrets[i]), writtenSecrets); if (isBatch) { if (liveRows.length === 0) { result = []; @@ -15761,6 +15846,10 @@ export class ObjectQL implements IObjectQLEngine { 'Insert operation failed', writeFailureLogMeta(redactBoundStatement(logged), { object }), ); + // [#27495] A write that did not complete takes back the credential rows + // it wrote; one a stored row names after all (a batch the driver + // partially applied, an after-hook refusal of a landed row) is kept. + await this.releaseSecretHandles(object, writtenSecrets, opCtx.context, 'refused'); throw e; } }); @@ -16796,6 +16885,12 @@ export class ObjectQL implements IObjectQLEngine { reportDroppedFields(preComputedPayload, computedStripUpdate.rows[0] as Record | null | undefined, 'computed'); hookContext.input.options = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any, { recordedVerdict: opCtx.tenantLayer0Verdict }); + // [#27495] The credential rows this write seals, the ones it has written + // to `sys_secret`, and the handles its committed statement moved the + // secret columns off. + let sealedSecrets: PendingSecretRow[] = []; + const writtenSecrets: PendingSecretRow[] = []; + let retiredSecrets: SecretHandleSlot[] = []; try { let result; @@ -16908,7 +17003,7 @@ export class ObjectQL implements IObjectQLEngine { // [#8559] Password half of the credential write door, then the // secret channel (which carries the secret-arm refusal). this.refuseEmptyPasswordFields(object, hookContext.input.data as Record); - await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); + sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); // [#21663] Scope `'skip'` — the public `validateRecord` IS that // scope: the readonly strip has NOT run yet, so a readonly value // here may be a caller's the strip is about to drop — judged, a @@ -17220,6 +17315,10 @@ export class ObjectQL implements IObjectQLEngine { // dispatch and the roll-up recompute, so an envelope applied // there would attribute a violation raised by a nested driver // call inside a hook to THIS object. + // [#27495] The credential row is written now, after every pass that + // can refuse this write and immediately before the statement; a + // statement that fails takes it back (the outer `catch`). + await this.persistSecretRows(sealedSecrets, writtenSecrets); try { // [ADR-0142 D4] A write under a precondition is the driver's // conditional update plus a readback — see @@ -17240,6 +17339,9 @@ export class ObjectQL implements IObjectQLEngine { // `data.record.updated` event and the door's ingress strip. A // `null` readback (the row left the caller's scope) stays `null`. result = withDeclaredColumnsOnly(result, declaredColumnSet(updateSchema)); + // [#27495] The handle the stored row named before, where this + // statement cleared or replaced it. + retiredSecrets = retiredSecretSlots(updateSchema, [priorRecord], hookContext.input.data); } else { // [#6262] A bulk SET clause must not carry `id`. Reaching this // branch AT ALL means `resolveEngineUpdateDispatch` returned @@ -17298,7 +17400,7 @@ export class ObjectQL implements IObjectQLEngine { // [#8559] Password half of the credential write door, then the // secret channel (which carries the secret-arm refusal). this.refuseEmptyPasswordFields(object, hookContext.input.data as Record); - await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); + sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); // [#21663] Scope `'skip'` — the public `validateRecord` IS that // scope: the readonly strip has NOT run yet, so a readonly value // here may be a caller's the strip is about to drop — judged, a @@ -17520,11 +17622,11 @@ export class ObjectQL implements IObjectQLEngine { // changes a value before the statement. The seam judges the rows // that will be stored, which is the rule the insert seam was // held to. The credential channel (`encryptSecretFields`) runs - // above on this branch, so a refused write that carried a secret - // field has already minted its `sys_secret` row. A validation - // refusal two lines down already pays the same cost here, and - // moving that channel is a separate change. A `check` naming a - // secret field judges the stored reference. + // above on this branch and only SEALS the `sys_secret` row there; + // the row is written immediately before the statement (#27495), + // so a write this seam or the validation below refuses leaves + // none. A `check` naming a secret field judges the stored + // reference. // // `honoured` is set BEFORE `evaluate`, exactly as on the insert // path: it answers "did the seam run", never "did the write @@ -17620,12 +17722,25 @@ export class ObjectQL implements IObjectQLEngine { const payload = hookContext.input.data as Record; summaryRows = { previous, next: previous.map((row) => ({ ...row, ...payload })) }; } + // [#27495] The handles the matched rows hold in the secret columns + // this payload moves, read BEFORE the statement replaces them: + // the D7 memo when a consumer above already made it, otherwise + // only those columns, through this write's own AST and options. + // A payload that moves no secret column reads nothing here. + const movedSecretColumns = secretFieldsWritten(updateSchema, hookContext.input.data); + const priorSecretRows = movedSecretColumns.length === 0 + ? [] + : priorRowsRead + ? ((await readPriorRows()) ?? []) + : (((await driver.find(object, { ...ast, fields: ['id', ...movedSecretColumns] }, hookContext.input.options as Parameters[2])) ?? []) as Record[]); + await this.persistSecretRows(sealedSecrets, writtenSecrets); try { result = await driver.updateMany!(object, ast, hookContext.input.data as Record, hookContext.input.options as any); } catch (driverError) { throw envelopeDriverWriteRefusal(driverError, object, this.getSchema(object), 'update', updateMsgCtx); } isPredicateWrite = true; + retiredSecrets = retiredSecretSlots(updateSchema, priorSecretRows, hookContext.input.data); } hookContext.event = 'afterUpdate'; @@ -17766,6 +17881,12 @@ export class ObjectQL implements IObjectQLEngine { // reason as the insert path: a strip HERE also fired on writes no // external caller ever sees). The generic READ path is unchanged: // {@link omitInternalFields} still runs on every find/findOne. + // [#27495] The write is complete: release the handles its statement + // moved the secret columns off, each only once storage confirms no + // row of its column still names it. After the `afterUpdate` chain, so + // a refusal that rolls the record back never reaches here, and before + // the roll-up verdict below, which reports a write that did land. + await this.releaseSecretHandles(object, retiredSecrets, opCtx.context, 'retired'); // The record IS updated; a summary that could not recompute after // retries must surface, not stay silent (framework#3147). if (summaryFailures.length > 0) throw new SummaryRecomputeError(summaryFailures, hookContext.result); @@ -17792,6 +17913,9 @@ export class ObjectQL implements IObjectQLEngine { 'Update operation failed', writeFailureLogMeta(redactBoundStatement(logged), { object }), ); + // [#27495] The insert door's twin: a write that did not complete takes + // back the credential row it wrote, unless a stored row names it. + await this.releaseSecretHandles(object, writtenSecrets, opCtx.context, 'refused'); throw e; } }); diff --git a/packages/objectql/src/secret-field-write-commit.test.ts b/packages/objectql/src/secret-field-write-commit.test.ts new file mode 100644 index 00000000000..d9d93b4f465 --- /dev/null +++ b/packages/objectql/src/secret-field-write-commit.test.ts @@ -0,0 +1,374 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27495] A `secret` field's `sys_secret` row exists only while a committed + * business row names it. + * + * Two halves, both on the credential channel (`encryptSecretFields` and the + * write steps around it), driven through a real {@link ObjectQL} over a stub + * driver that keeps every store in memory, so the `sys_secret` table can be + * counted after each write: + * + * - **Written only with the record.** The credential pass seals the value + * where it always has (the ref is on the row before validation judges it), + * but the `sys_secret` row is written immediately before the record's own + * driver write, and taken back if that write does not land. A write refused + * for any reason before then (validation, an option outside the list, a + * required field, a `before*` hook, the post-image permission seam) writes + * nothing; a driver that refuses the statement leaves nothing either. + * - **Released when the column lets go.** A committed update that clears the + * field (`null`) or replaces its value releases the handle the column named + * before, once storage confirms no row of that column still names it and + * the handle is this column's own. A handle a predicate write shared across + * rows outlives every row but the last. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import type { Hook } from '@objectstack/spec/data'; +import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts'; +import { ObjectQL } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; +import { SECRET_MASK, isSecretRef, parseSecretRef } from './secret-fields.js'; + +type Row = Record; + +/** A stub driver whose stores can be counted, rolled back and told to refuse a statement. */ +function makeDriver() { + let stores = new Map>(); + const storeFor = (o: string) => stores.get(o) ?? (stores.set(o, new Map()), stores.get(o)!); + const matches = (row: Row, where: any) => + !where || Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) return true; + if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(row[k]); + return (row[k] ?? null) === (v ?? null); + }); + const copy = (r: Row | undefined) => (r == null ? r : { ...r }); + /** Objects whose next record statement the driver refuses (create / update / updateMany). */ + const refuse = new Set(); + /** Objects whose `count` throws, so a release cannot confirm anything. */ + const countFails = new Set(); + const snapshots = new Map(); + let n = 0; + const refused = (o: string) => { + if (refuse.has(o)) throw Object.assign(new Error(`statement refused on ${o}`), { code: 'ETIMEDOUT' }); + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + const page = typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + return page.map(copy); + }, + async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; }, + async create(o: string, data: Row) { + refused(o); + const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` }; + storeFor(o).set(String(row.id), row); + return copy(row); + }, + async update(o: string, id: string, data: Row) { + refused(o); + const row = { ...storeFor(o).get(String(id)), ...data, id }; + storeFor(o).set(String(id), row); + return copy(row); + }, + async updateMany(o: string, ast: any, data: Row) { + refused(o); + let k = 0; + for (const [id, row] of storeFor(o)) { + if (!matches(row, ast?.where)) continue; + storeFor(o).set(id, { ...row, ...data, id }); + k += 1; + } + return k; + }, + async delete(o: string, id: string) { return storeFor(o).delete(String(id)); }, + async count(o: string, ast: any) { + if (countFails.has(o)) throw new Error(`count unavailable on ${o}`); + return (await this.find(o, ast)).length; + }, + async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { + const handle = { trx: snapshots.size + 1 }; + snapshots.set(handle, new Map(Array.from(stores, ([o, s]) => [o, new Map(Array.from(s, ([k, v]) => [k, { ...v }]))]))); + return handle; + }, + async commit(h: unknown) { snapshots.delete(h); }, + async rollback(h: unknown) { stores = snapshots.get(h) ?? stores; snapshots.delete(h); }, + }; + return { + driver, refuse, countFails, + rowsOf: (o: string) => Array.from(storeFor(o).values()), + }; +} + +function makeCrypto(): ICryptoProvider { + let n = 0; + return { + async encrypt(plain: string): Promise { + n += 1; + return { id: `sec_${n}`, kmsKeyId: 'local', alg: 'test-b64', version: 1, ciphertext: Buffer.from(plain).toString('base64') }; + }, + async decrypt(handle: CryptoHandle) { return Buffer.from(handle.ciphertext, 'base64').toString('utf8'); }, + async rotateKey(handle: CryptoHandle) { return handle; }, + digest(plain: string) { return `d:${plain.length}`; }, + async keyedDigest(plain: string) { return `k:${plain.length}`; }, + }; +} + +const sysSecret = { + name: 'sys_secret', label: 'Secret', systemFields: { tenant: false }, + fields: { + id: { name: 'id', type: 'text' }, namespace: { name: 'namespace', type: 'text' }, key: { name: 'key', type: 'text' }, + kms_key_id: { name: 'kms_key_id', type: 'text' }, alg: { name: 'alg', type: 'text' }, + version: { name: 'version', type: 'number' }, ciphertext: { name: 'ciphertext', type: 'text' }, + created_at: { name: 'created_at', type: 'datetime' }, + }, +}; +const VAULT = 'qa_vault'; +const vault = { + name: VAULT, label: 'Vault', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text', required: true }, + tier: { name: 'tier', type: 'select', options: [{ label: 'Gold', value: 'gold' }, { label: 'Silver', value: 'silver' }] }, + token: { name: 'token', type: 'secret' }, + }, +}; +const OTHER = 'qa_other'; +const other = { + name: OTHER, label: 'Other', + fields: { id: { name: 'id', type: 'text' }, name: { name: 'name', type: 'text' }, other_token: { name: 'other_token', type: 'secret' } }, +}; + +const SYSTEM = { context: { isSystem: true } } as any; + +describe('[#27495] a sys_secret row exists only while a committed row names it', () => { + let engine: ObjectQL; + let d: ReturnType; + let errors: string[]; + let logger: any; + /** What the post-image permission seam does on the next write: let it through, or refuse it. */ + let seam: 'none' | 'refuse'; + + const secrets = () => d.rowsOf('sys_secret'); + const stored = (id: string) => d.rowsOf(VAULT).find((r) => r.id === id) as Row; + const handleOf = (id: string) => parseSecretRef(stored(id).token); + const seed = async (id: string, token: string) => { + await engine.insert(VAULT, { id, name: id, tier: 'gold', token }, SYSTEM); + return handleOf(id)!; + }; + + beforeEach(async () => { + errors = []; + seam = 'none'; + logger = { + error: (m: string) => errors.push(String(m)), + warn() {}, debug() {}, info() {}, trace() {}, fatal() {}, + child() { return logger; }, + }; + engine = new ObjectQL({ logger }); + d = makeDriver(); + engine.registerDriver(d.driver, true); + await engine.init(); + engine.registry.registerObject(sysSecret as any); + engine.registry.registerObject(vault as any); + engine.registry.registerObject(other as any); + engine.setCryptoProvider(makeCrypto()); + engine.registerMiddleware(async (opCtx: any, next: () => Promise) => { + if (seam === 'refuse' && (opCtx.operation === 'insert' || opCtx.operation === 'update')) { + opCtx.postHookWriteImageCheck = { + evaluate: () => { throw Object.assign(new Error('write check refused'), { code: 'PERMISSION_DENIED', status: 403 }); }, + }; + } + await next(); + }); + }); + + describe('a refused write writes no sys_secret row', () => { + it('insert refused by a required field', async () => { + await expect(engine.insert(VAULT, { token: 'orphan-1' })).rejects.toMatchObject({ + code: 'VALIDATION_FAILED', fields: [expect.objectContaining({ field: 'name', code: 'required' })], + }); + expect(d.rowsOf(VAULT)).toHaveLength(0); + expect(secrets()).toHaveLength(0); + }); + + it('insert refused by an option outside the list', async () => { + await expect(engine.insert(VAULT, { name: 'x', tier: 'bogus', token: 'orphan-2' })).rejects.toMatchObject({ + code: 'VALIDATION_FAILED', fields: [expect.objectContaining({ field: 'tier', code: 'invalid_option' })], + }); + expect(secrets()).toHaveLength(0); + }); + + it('insert refused by a beforeInsert hook', async () => { + engine.registerHook('beforeInsert', () => { throw Object.assign(new Error('hook says no'), { code: 'HOOK_REFUSED', status: 400 }); }, { object: VAULT }); + await expect(engine.insert(VAULT, { name: 'x', token: 'orphan-3' })).rejects.toMatchObject({ code: 'HOOK_REFUSED', status: 400 }); + expect(secrets()).toHaveLength(0); + }); + + it('insert refused by the post-image permission seam', async () => { + seam = 'refuse'; + await expect(engine.insert(VAULT, { name: 'x', token: 'orphan-4' })).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(secrets()).toHaveLength(0); + }); + + it('insert whose record statement the driver refuses', async () => { + d.refuse.add(VAULT); + await expect(engine.insert(VAULT, { name: 'x', token: 'orphan-5' })).rejects.toThrow(/statement refused/); + expect(d.rowsOf(VAULT)).toHaveLength(0); + expect(secrets()).toHaveLength(0); + expect(errors).toEqual([]); + }); + + it('a batch insert whose statement the driver refuses', async () => { + d.refuse.add(VAULT); + await expect(engine.insert(VAULT, [{ name: 'a', token: 'b-1' }, { name: 'b', token: 'b-2' }])).rejects.toThrow(/statement refused/); + expect(secrets()).toHaveLength(0); + }); + + it('partial insertMany: only the row that lands keeps a credential row', async () => { + const outcomes = await engine.insertMany(VAULT, [{ token: 'dead' }, { name: 'alive', token: 'kept' }]); + expect(outcomes.map((o) => o.ok)).toEqual([false, true]); + expect(secrets()).toHaveLength(1); + const [alive] = d.rowsOf(VAULT); + expect(parseSecretRef(alive.token)).toBe(secrets()[0].id); + expect(await engine.resolveSecretField(VAULT, String(alive.id), 'token')).toBe('kept'); + }); + + it('update refused by an option outside the list keeps the old secret and writes no new row', async () => { + const before = await seed('r1', 'stays'); + await expect(engine.update(VAULT, { id: 'r1', tier: 'bogus', token: 'orphan-6' })).rejects.toMatchObject({ + code: 'VALIDATION_FAILED', fields: [expect.objectContaining({ field: 'tier', code: 'invalid_option' })], + }); + expect(secrets().map((s) => s.id)).toEqual([before]); + expect(handleOf('r1')).toBe(before); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('stays'); + }); + + it('update refused by the post-image permission seam', async () => { + const before = await seed('r1', 'stays'); + seam = 'refuse'; + await expect(engine.update(VAULT, { id: 'r1', token: 'orphan-7' })).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(secrets().map((s) => s.id)).toEqual([before]); + }); + + it('update whose record statement the driver refuses', async () => { + const before = await seed('r1', 'stays'); + d.refuse.add(VAULT); + await expect(engine.update(VAULT, { id: 'r1', token: 'orphan-8' })).rejects.toThrow(/statement refused/); + expect(secrets().map((s) => s.id)).toEqual([before]); + expect(handleOf('r1')).toBe(before); + }); + + it('a predicate update refused by an option outside the list', async () => { + const before = [await seed('r1', 'one'), await seed('r2', 'two')]; + await expect( + engine.update(VAULT, { tier: 'bogus', token: 'orphan-9' }, { where: { tier: 'gold' }, multi: true } as any), + ).rejects.toMatchObject({ code: 'VALIDATION_FAILED' }); + expect(secrets().map((s) => s.id).sort()).toEqual(before.sort()); + }); + + it('a predicate update whose statement the driver refuses', async () => { + const before = [await seed('r1', 'one'), await seed('r2', 'two')]; + d.refuse.add(VAULT); + await expect( + engine.update(VAULT, { token: 'orphan-10' }, { where: { tier: 'gold' }, multi: true } as any), + ).rejects.toThrow(/statement refused/); + expect(secrets().map((s) => s.id).sort()).toEqual(before.sort()); + }); + + it('an aborting afterInsert rolls the credential row back with the record', async () => { + bindHooksToEngine(engine, [ + { name: 'veto', object: VAULT, events: ['afterInsert'], handler: () => { throw new Error('refused by the hook'); } } as unknown as Hook, + ], { packageId: 'app:t', logger }); + await expect(engine.insert(VAULT, { name: 'x', token: 'orphan-11' })).rejects.toThrow(/refused by the hook/); + expect(d.rowsOf(VAULT)).toHaveLength(0); + expect(secrets()).toHaveLength(0); + }); + }); + + describe('a committed write still stores and reads back', () => { + it('insert seals the value, masks it on read and dereferences it', async () => { + const created = await engine.insert(VAULT, { name: 'ok', token: 'kept' }); + const row = stored(String(created.id)); + expect(isSecretRef(row.token)).toBe(true); + expect(secrets()).toHaveLength(1); + expect(secrets()[0]).toMatchObject({ id: parseSecretRef(row.token), namespace: VAULT, key: 'token' }); + expect(((await engine.findOne(VAULT, { where: { id: created.id } })) as Row).token).toBe(SECRET_MASK); + expect(await engine.resolveSecretField(VAULT, String(created.id), 'token')).toBe('kept'); + }); + + it('an echoed mask changes nothing and releases nothing', async () => { + const before = await seed('r1', 'stays'); + await engine.update(VAULT, { id: 'r1', name: 'renamed', token: SECRET_MASK }); + expect(handleOf('r1')).toBe(before); + expect(secrets().map((s) => s.id)).toEqual([before]); + }); + }); + + describe('a committed write releases the handle its column let go of', () => { + it('clearing with null removes the old row', async () => { + await seed('r1', 'gone'); + await engine.update(VAULT, { id: 'r1', token: null }); + expect(stored('r1').token ?? null).toBeNull(); + expect(secrets()).toHaveLength(0); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBeNull(); + }); + + it('replacing the value removes the old row and keeps the new one', async () => { + const before = await seed('r1', 'old'); + await engine.update(VAULT, { id: 'r1', token: 'new' }); + expect(handleOf('r1')).not.toBe(before); + expect(secrets().map((s) => s.id)).toEqual([handleOf('r1')]); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('new'); + }); + + it('a predicate clear removes every matched row\'s handle', async () => { + await seed('r1', 'one'); + await seed('r2', 'two'); + await engine.update(VAULT, { token: null }, { where: { tier: 'gold' }, multi: true } as any); + expect(secrets()).toHaveLength(0); + }); + + it('a handle a predicate write shared across rows outlives every row but the last', async () => { + await seed('r1', 'one'); + await seed('r2', 'two'); + await engine.update(VAULT, { token: 'shared' }, { where: { tier: 'gold' }, multi: true } as any); + const shared = handleOf('r1'); + expect(handleOf('r2')).toBe(shared); + expect(secrets().map((s) => s.id)).toEqual([shared]); + + await engine.update(VAULT, { id: 'r1', token: null }); + expect(secrets().map((s) => s.id)).toEqual([shared]); + expect(await engine.resolveSecretField(VAULT, 'r2', 'token')).toBe('shared'); + + await engine.update(VAULT, { id: 'r2', token: null }); + expect(secrets()).toHaveLength(0); + }); + + it('a handle minted for another column is never released through this one', async () => { + const foreign = await seed('r1', 'theirs'); + // A ref a system writer copied onto another object's column: that column + // letting go of it says nothing about the column the handle was minted for. + await engine.insert(OTHER, { id: 'o1', name: 'copy', other_token: `secret:${foreign}` }, SYSTEM); + await engine.update(OTHER, { id: 'o1', other_token: null }); + expect(secrets().map((s) => s.id)).toEqual([foreign]); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('theirs'); + }); + + it('a release that cannot confirm the column let go keeps the row and says so', async () => { + const before = await seed('r1', 'stays-stored'); + d.countFails.add(VAULT); + await engine.update(VAULT, { id: 'r1', token: null }); + expect(stored('r1').token ?? null).toBeNull(); + expect(secrets().map((s) => s.id)).toEqual([before]); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain(`${VAULT}.token`); + expect(errors[0]).not.toContain(before); + }); + }); +}); diff --git a/packages/qa/dogfood/test/secret-field-orphan-rows.dogfood.test.ts b/packages/qa/dogfood/test/secret-field-orphan-rows.dogfood.test.ts new file mode 100644 index 00000000000..7a4d6b05b72 --- /dev/null +++ b/packages/qa/dogfood/test/secret-field-orphan-rows.dogfood.test.ts @@ -0,0 +1,141 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#27495] A `secret` field's `sys_secret` row exists only while a committed +// business row names it — pinned at the REST data door of a booted stack, on +// a provider-wired boot (`bootStack` wires one LocalCryptoProvider, as +// `objectstack dev` / `serve` do). `packages/objectql/src/secret-field-write-commit.test.ts` +// drives every refusal class at the engine; this file holds the same two +// halves through the door a client actually writes through: +// +// - a write the door refuses (a required field missing, an option outside the +// list, on create and on update) leaves the `sys_secret` row count as it was; +// - a PATCH that clears the field with `null`, or replaces its value, removes +// the row the field named before, and a committed write still stores and +// reads back. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { SECRET_MASK } from '@objectstack/objectql'; + +const VAULT = 'sfo_vault'; + +const sfoStack = defineStack({ + manifest: { + id: 'com.dogfood.secret-field-orphan-rows', namespace: 'sfo', version: '0.0.0', type: 'app', + name: 'Secret field orphan-row fixture', description: 'A secret field beside a required field and a picklist.', + }, + objects: [ + ObjectSchema.create({ + name: VAULT, label: 'Vault', pluralLabel: 'Vaults', sharingModel: 'public_read_write', + fields: { + name: Field.text({ label: 'Name', required: true }), + tier: Field.select({ label: 'Tier', options: [{ label: 'Gold', value: 'gold' }, { label: 'Silver', value: 'silver' }] }), + token: Field.secret({ label: 'Token' }), + }, + }), + ], +} as never); + +type Answer = { status: number; json: any; text: string }; + +describe('[#27495] secret field: a sys_secret row only while a committed row names it — over REST', () => { + let stack: VerifyStack; + let ql: any; + let token = ''; + + const call = async (method: string, path: string, body?: unknown): Promise => { + const res = await stack.apiAs(token, method, path, body); + const text = await res.text(); + let json: any = {}; + try { json = JSON.parse(text); } catch { /* not JSON */ } + return { status: res.status, json, text }; + }; + /** Every credential row the store holds, read at the driver (no engine read path between). */ + const secretRows = async (): Promise>> => + (await ql.getDriver('sys_secret').find('sys_secret', {})) ?? []; + /** The ref the business row holds, as the driver stores it. */ + const storedRef = async (id: string) => { + const [row] = await ql.getDriver(VAULT).find(VAULT, { where: { id } }); + return row?.token as string | null | undefined; + }; + const create = async (name: string, value: string) => { + const res = await call('POST', `/data/${VAULT}`, { name, tier: 'gold', token: value }); + expect(res.status, res.text).toBe(201); + const [row] = await stack.rows(VAULT, { name }); + return String(row.id); + }; + + beforeAll(async () => { + stack = await bootStack(sfoStack); + token = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('a create refused for a missing required field or an option outside the list writes no credential row', async () => { + const before = (await secretRows()).length; + const missing = await call('POST', `/data/${VAULT}`, { tier: 'gold', token: 'sfo-refused-1' }); + expect(missing.status, missing.text).toBe(400); + expect(missing.json.code, missing.text).toBe('VALIDATION_FAILED'); + const bogus = await call('POST', `/data/${VAULT}`, { name: 'sfo-bogus', tier: 'bogus', token: 'sfo-refused-2' }); + expect(bogus.status, bogus.text).toBe(400); + expect(bogus.json.code, bogus.text).toBe('VALIDATION_FAILED'); + expect(await stack.rows(VAULT, { name: 'sfo-bogus' })).toHaveLength(0); + expect((await secretRows()).length).toBe(before); + }); + + it('an update refused for an option outside the list keeps the stored secret and writes no credential row', async () => { + const id = await create('sfo-keep', 'sfo-kept-value'); + const ref = await storedRef(id); + const before = (await secretRows()).length; + const refused = await call('PATCH', `/data/${VAULT}/${id}`, { tier: 'bogus', token: 'sfo-refused-3' }); + expect(refused.status, refused.text).toBe(400); + expect(refused.json.code, refused.text).toBe('VALIDATION_FAILED'); + expect((await secretRows()).length).toBe(before); + expect(await storedRef(id)).toBe(ref); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sfo-kept-value'); + }); + + it('a committed create stores the secret, masks it on read and dereferences it', async () => { + const before = (await secretRows()).length; + const id = await create('sfo-ok', 'sfo-stored-value'); + expect(String(await storedRef(id))).toMatch(/^secret:/); + expect((await secretRows()).length).toBe(before + 1); + const read = await call('GET', `/data/${VAULT}/${id}`); + expect(read.status, read.text).toBe(200); + expect((read.json.record ?? read.json).token).toBe(SECRET_MASK); + expect(read.text).not.toContain('sfo-stored-value'); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sfo-stored-value'); + }); + + it('clearing the field with null removes the credential row it named', async () => { + const id = await create('sfo-clear', 'sfo-cleared-value'); + const handle = String(await storedRef(id)).slice('secret:'.length); + expect((await secretRows()).some((r) => r.id === handle)).toBe(true); + const cleared = await call('PATCH', `/data/${VAULT}/${id}`, { token: null }); + expect(cleared.status, cleared.text).toBe(200); + expect(await storedRef(id) ?? null).toBeNull(); + expect((await secretRows()).some((r) => r.id === handle)).toBe(false); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBeNull(); + }); + + it('replacing the value removes the old credential row and keeps the new one', async () => { + const id = await create('sfo-replace', 'sfo-old-value'); + const oldHandle = String(await storedRef(id)).slice('secret:'.length); + const before = (await secretRows()).length; + const replaced = await call('PATCH', `/data/${VAULT}/${id}`, { token: 'sfo-new-value' }); + expect(replaced.status, replaced.text).toBe(200); + const newHandle = String(await storedRef(id)).slice('secret:'.length); + expect(newHandle).not.toBe(oldHandle); + const after = await secretRows(); + expect(after.length).toBe(before); + expect(after.some((r) => r.id === oldHandle)).toBe(false); + expect(after.some((r) => r.id === newHandle)).toBe(true); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sfo-new-value'); + }); +});