diff --git a/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md b/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md new file mode 100644 index 00000000000..89e5482aa75 --- /dev/null +++ b/.changeset/update-stored-row-caller-read-secret-ref-and-repoint.md @@ -0,0 +1,16 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: an update compares a value the caller sends only against the stored row as the caller's own read door serves it — a `secret` reference and a master-detail repoint + +Clause-②: yes (narrowing) — a non-system update that sends a `secret:` reference is refused even when the record already holds it, and a non-system update naming the master-detail FK of a detail with a parent-scoped `requiredWhen`, in a FK column it may edit but not read, no longer lets a row that already lacks a required value rest under the header it names. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at two update seams of `@objectstack/objectql`, served by every write door, by id and in bulk alike. A system write is unchanged at both. + +**A `secret` reference.** A non-system update keeps a `secret:` reference only where every record it lands on serves that reference, in that column, to the caller's own read door. The read path serves a `secret` column masked to every caller, so after the upgrade every non-system update that sends a reference is refused with `400 VALIDATION_ERROR` and nothing is written, with one answer whatever the reference names and whatever the column holds — the record's own reference included, which was accepted and kept before. The same holds for a reference a `beforeUpdate` hook writes onto the payload, such as one that spreads the stored row (`ctx.previous`) back into it. To leave a secret unchanged, omit the field or send back the read mask; a hook that writes the stored row back omits its `secret` columns. A new value is still sealed, `null` still clears, and the system principal keeps the pass-through. + +**A master-detail repoint.** On a detail whose fields declare a parent-scoped `requiredWhen`, the ADR-0113 pre-check of an update that names the master-detail FK reads the header the row hung off from the stored FK only as the caller's own read door serves it. Where the caller may edit the FK but not read it (a field permission `{ readable: false, editable: true }`), the prior header is unknown to it and is bound as an unresolved one, so the pre-check cannot prove a violation was already stored: a row that already lacks a value its requirement asks for, and that such a caller names a header for, is refused with the field's `required` finding whatever header the row hung off — including the header it already hangs off, which used to rest. A caller who can read the FK is judged as before, and an update that does not name the FK is unchanged. The `validate()` preview of such an update binds the prior header the same way, so it predicts the same `required` refusal instead of admitting the row. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 06f95ab9505..f0278872e85 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -138,7 +138,7 @@ bugs — it is the protection or the side effect that silently does not happen. | 23b | **Position-catalog check skipped** — a `sys_user_position` write whose `position` names no position in the security catalog is not refused | plugin-security | Get: a system writer can store an assignment naming no position — the seed loader writes `stack.data` from `AppPlugin.start()` under a system context, as invitation acceptance and the platform's own bootstraps do. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no position of that name: the environment registry (ADR-0131 D3), one per deployment, which takes no organization, so a name only a `sys_position` row carries — any organization's — is refused like one nobody carries. It is the same stand-down as the engine's referential-integrity check for a lookup column | `assertPositionNamesCatalogRow` | | 23c | **Grant-name catalog check skipped** — a `sys_user_permission_set` write whose `permission_set` names no permission set in the security catalog is not refused | plugin-security | Get: a system writer can store a grant naming a set not yet registered — seed replay and boot provisioning may write the grant before the definition it names is registered. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `permission_set`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no set by that name; a grant that names no set at all is refused `required` for every caller. The grant names its set by name alone, and nothing derives the name from `permission_set_id` (ADR-0131 D4, D10) | `packages/plugins/plugin-security/src/grant-permission-set-name.ts#onInsert`, `packages/plugins/plugin-security/src/grant-permission-set-name.ts#onUpdate` | | 23d | **Grant-holder membership check skipped** — a `sys_user_position` or `sys_user_permission_set` write naming a user with no `sys_member` row in the row's organization is not refused | plugin-security | Get: a system writer can store such a grant — invitation acceptance writes the membership and its placement together, and seed replay, the organization-admin reconcile and the platform bootstraps write grants as the system. Lose: such a row takes effect once its user is admitted to the organization. A non-system insert, or a non-system update that moves `user_id` or `organization_id`, is refused `400 VALIDATION_FAILED` / `reference_not_found` at `user_id` instead, for every caller, a platform administrator included; the membership is read as `{ isSystem: true, tenantId }` with `tenantId` the row's own organization, never another one. A row stored with no organization is not judged. Under `single`, 18.0 stamps a new grant with the Default Organization, so such a row is one an earlier release wrote | `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onInsert`, `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onUpdate` | -| 23e | **A `secret` field takes a stored reference as given** — a value already spelled `secret:` is stored unchanged | objectql | Get: a system writer may copy a `secret:` reference onto a record as it stands (seed replay, a migration). Every other caller may only seal a new value or keep its own record's ref: on a create any such value is refused, and on an update it is accepted only when every record the write lands on already holds exactly that reference in that column, which is then left as it is, as an echoed read mask is. Any other reference is refused `400 VALIDATION_ERROR` before anything is written, with one answer whatever it names. Lose: the check that the reference is the record's own — a system writer that copies one is trusted to mean it. Inbound HTTP never carries the flag | `encryptSecretFields` | +| 23e | **A `secret` field takes a stored reference as given** — a value already spelled `secret:` is stored unchanged | objectql | Get: a system writer may copy a `secret:` reference onto a record as it stands (seed replay, a migration). Every other caller may only seal a new value or keep its own record's ref, and its own is the value it reads in that column: on a create any such value is refused, and on an update it would be kept only where every record the write lands on serves exactly that reference to this caller's own read door. The read path serves a `secret` column masked to every caller, so every reference such a caller sends is refused `400 VALIDATION_ERROR` before anything is written, with one answer whatever it names and whatever the column holds; an echoed read mask still leaves the column as it is. Lose: the check that the reference is the record's own — a system writer that copies one is trusted to mean it. Inbound HTTP never carries the flag | `encryptSecretFields` | | 23f | **Agent ceiling grant check skipped** — a `sys_user_permission_set` write naming an MCP agent ceiling set is not refused | plugin-security | Get: a system writer is not judged. The platform-admin promotion and the organization-admin reconcile name their sets in code, and neither is a ceiling; the self-registration grant writes the configured `audience.selfRegistrationPermissionSet`, which the audience validation (`assertAudienceConfig`, plugin-auth) refuses when it names an agent ceiling set. Lose: seed replay and the development-only demo-staff seed write the sets their stack declares, unjudged, so a seed that names an agent ceiling set stores it. A non-system insert, or a non-system update whose resulting row names one, is refused `400 VALIDATION_FAILED` / `invalid_value` at `permission_set`, for every caller, a platform administrator included; a delete is not judged (ADR-0090 D10) | `packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.ts#onInsert`, `packages/plugins/plugin-security/src/agent-ceiling-grant-refusal.ts#onUpdate` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `buildDriverOptions` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `packages/plugins/plugin-security/src/system-write-guard.ts#assertEngineOwnedWriteAllowed` | diff --git a/packages/objectql/src/engine-master-detail-related.ts b/packages/objectql/src/engine-master-detail-related.ts index ef2d03d2428..609f4c40dee 100644 --- a/packages/objectql/src/engine-master-detail-related.ts +++ b/packages/objectql/src/engine-master-detail-related.ts @@ -54,6 +54,11 @@ * onto its function (`--hand-docs` of `scripts/codemod-split-engine.mjs`), * and the class keeps none for it; a public or protected method's docblock * stays on the class, the contract, and its function keeps the placeholder. + * + * One body was written here rather than moved: `priorMasterIdsCallerReads`, + * the stored master an update's pre-check reads, as the caller's read door + * serves the FK. Its private method on the class is a delegation of the same + * shape. */ import { referenceTargetOf } from '@objectstack/spec/data'; @@ -70,6 +75,7 @@ import type { RelatedFieldBinding, RelatedRecordBinding } from './validation/rul import { resolveMasterDetailRelation } from './master-detail.js'; import { isFederatedObject } from './federated-object.js'; import { masterIdOf, servedAsStored, staticReadonlyStoredView, undeclaredRelatedFields } from './engine.js'; +import { storedReferencesCallerReads } from './engine-referential-integrity.js'; import type { ObjectQL } from './engine.js'; /** @@ -248,6 +254,14 @@ export async function settleMasterDetailLanding( * merged with the patch on an update, so the patch's FK wins over the stored * one; `storedRows[i]` names the header a repoint leaves. * + * ⛔ `storedRows[i]` holds only the columns the read door served this caller + * as stored, so a stored FK it was not served is absent there. When the patch + * names the FK over such a row, the header the row leaves is UNKNOWN to this + * caller, and `previousParents[i]` is `null`, the unresolved header — the one + * fixed binding the write gives it ({@link priorMasterIdsCallerReads}), so the + * pre-check's verdict is the write's and never depends on what the column + * holds. A patch that does not name the FK reads no second header. + * * ## Read under the caller's access, not elevated * * A header read elevated here would answer a predicate over a row the caller @@ -290,6 +304,13 @@ export async function resolvePreviewParents( const priorIds = storedRows.map((stored) => (rel && stored ? masterIdOf(rel.fk, null, stored) : undefined)); // The write resolves a second header only for a repoint (`repointsMaster`). const repoints = priorIds.map((prior, i) => prior != null && landingIds[i] !== prior); + // The patch names the FK over a stored row whose FK this caller was not + // served as stored: the prior header is unknown, as the write binds it. + const priorUnknown = storedRows.map((stored, i) => { + const view = views[i]; + return rel != null && stored != null && !Object.prototype.hasOwnProperty.call(stored, rel.fk) + && view != null && typeof view === 'object' && rel.fk in view; + }); const ids = new Set(); landingIds.forEach((id) => { if (id != null) ids.add(String(id)); }); priorIds.forEach((id, i) => { if (repoints[i]) ids.add(String(id)); }); @@ -334,10 +355,50 @@ export async function resolvePreviewParents( const headerOf = (id: string | number | undefined) => (id == null ? null : (headers.get(String(id)) ?? null)); return { parents: landingIds.map(headerOf), - previousParents: priorIds.map((id, i) => (repoints[i] ? headerOf(id) : undefined)), + previousParents: priorIds.map((id, i) => (priorUnknown[i] ? null : repoints[i] ? headerOf(id) : undefined)), }; } +/** + * The master each of an update's stored rows hangs off, as THIS caller's own + * read door serves the master-detail FK: the header the ADR-0113 pre-check of + * a parent-scoped `requiredWhen` reads as `previousParent` when the write + * names the FK. + * + * `{ id }` for a row whose FK the read door serves this caller exactly as + * stored ({@link storedReferencesCallerReads}, the update's one projection of + * stored rows onto the caller's read): `id` is the stored master, or + * `undefined` for a row that hangs off none. `null` for a row whose FK it does + * not — hidden, or served transformed, or a row the door does not return: the + * prior master is UNKNOWN to this caller, and the write binds one fixed header + * for it, the unresolved one, so neither whether the pre-check reads a second + * header nor which one it reads depends on what the column holds. A SYSTEM + * caller is answered from the rows as stored, as before. + * + * A stored row that carries no FK key at all is read as holding no master, as + * `masterIdOf` reads it: the key is spelled out as empty before the + * projection, so a system caller's answer for it is `{ id: undefined }`. + */ +export async function priorMasterIdsCallerReads( + this: Pick, + schema: unknown, + rows: ReadonlyArray | null | undefined>, + context: ExecutionContext | undefined, +): Promise> { + const rel = resolveMasterDetailRelation(schema as any); + if (!rel) return rows.map(() => null); + const stored = rows.map((row) => { + const own = row && typeof row === 'object' ? row : {}; + return Object.prototype.hasOwnProperty.call(own, rel.fk) ? own : { ...own, [rel.fk]: undefined }; + }); + const served = await storedReferencesCallerReads.call( + this, (schema as { name?: unknown } | null | undefined)?.name, stored, [rel.fk], context, + ); + return served.map((view) => ( + view && Object.prototype.hasOwnProperty.call(view, rel.fk) ? { id: masterIdOf(rel.fk, null, view) } : null + )); +} + /** * [#18682] Resolve the related rows this object's PREDICATE rules read one * hop through a reference field, for a whole batch of rows at once. diff --git a/packages/objectql/src/engine-secret-internal-fields.ts b/packages/objectql/src/engine-secret-internal-fields.ts index fe8ad2a06ca..2c8afad5a92 100644 --- a/packages/objectql/src/engine-secret-internal-fields.ts +++ b/packages/objectql/src/engine-secret-internal-fields.ts @@ -27,7 +27,11 @@ * may only seal a new value or keep its own record's ref. `encryptSecretFields` * takes the write's own stored rows ({@link StoredRowsReader}) and refuses any * other ref with {@link SecretRefWriteError}; the system principal keeps the - * pass-through. + * pass-through. The stored rows a non-system write is compared against are + * the rows as that caller's own read door serves them + * ({@link storedReferencesCallerReads}), which serves a `secret` column + * masked, so no stored ref is one a non-system write keeps: every ref it + * sends gets the same refusal, whatever the column holds. * * ## How the moved bodies reach the class * @@ -40,7 +44,7 @@ * (`_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`, + * reads, as it did. The seven methods they call (`find`, `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 @@ -74,6 +78,7 @@ import { SECRET_MASK, } from './secret-fields.js'; import { isSearchCompanionRequested, stripSearchCompanion } from './search-companion.js'; +import { storedReferencesCallerReads } from './engine-referential-integrity.js'; import type { ObjectQL } from './engine.js'; /** @@ -89,6 +94,7 @@ export interface SecretInternalFieldsHost { readonly cryptoProvider: ObjectQL['cryptoProvider']; readonly txStore: ObjectQL['txStore']; readonly logger: ObjectQL['logger']; + find: ObjectQL['find']; getDriver: ObjectQL['getDriver']; transactionCoversDriverFor: ObjectQL['transactionCoversDriverFor']; isClosedUnitHandle: ObjectQL['isClosedUnitHandle']; @@ -124,7 +130,9 @@ export interface SecretHandleSlot { * insert (a new record holds no ref of its own), the pre-image for a by-id * update, the matched rows for a predicate update — and the reader is called * only when a non-system payload carries a ref, so every other write reads - * nothing more than it did. + * nothing more than it did. The rows are compared only as the caller's own + * read door serves them ({@link storedReferencesCallerReads}), never as the + * call site read them. */ export type StoredRowsReader = () => Promise | null | undefined)[] | null>; @@ -140,9 +148,12 @@ export const SECRET_REF_WRITE_REFUSAL_STATUS = 400; /** * [#27582] Refusal of a `secret:` ref on a non-system write that is not the - * value the record's own column already holds. Fail closed, with one answer: - * the message names the column and never the ref, and nothing about the handle - * the ref names is looked up, so the answer is the same whatever it names. + * value the record's own column holds as its caller reads it. Fail closed, with + * one answer: the message names the column and never the ref, and nothing about + * the handle the ref names is looked up, so the answer is the same whatever it + * names. The read door serves a `secret` column masked to every caller, so a + * non-system write that sends a ref always meets this refusal, whatever the + * column holds. */ export class SecretRefWriteError extends Error { readonly code = SECRET_REF_WRITE_REFUSAL_CODE; @@ -154,9 +165,10 @@ export class SecretRefWriteError extends Error { constructor(object: string, field: string) { super( `Stored-reference value refused for secret field "${object}.${field}": a value that starts with "secret:" ` - + 'is read as a reference to a credential already stored, not as a credential, and a write may only keep ' - + 'the reference this record already holds. To set the secret, send its value; to clear it, write null; ' - + 'to leave it unchanged, omit the field. Nothing was written.', + + 'is read as a reference to a credential already stored, not as a credential, and a write keeps a ' + + 'reference only where it is the value its caller reads in this column, which serves a secret masked. ' + + 'To set the secret, send its value; to clear it, write null; to leave it unchanged, omit the field or ' + + 'send back the mask it reads. Nothing was written.', ); this.name = 'SecretRefWriteError'; this.object = object; @@ -200,18 +212,21 @@ export class SecretRefWriteError extends Error { * a new value or keep its own record's ref. From the system principal * (`context.isSystem === true`) it is left as-is, as it always was. From * any other caller it is kept only when every row the write lands on - * already holds exactly that ref in that column — read from `storedRows`, - * which each call site supplies (none for an insert, the pre-image for a - * by-id update, the matched rows for a predicate update) — and then the - * key is dropped, as the echoed mask is. Any other ref is REFUSED with + * holds exactly that ref in that column AS THE CALLER'S OWN READ DOOR + * SERVES IT — `storedRows`, which each call site supplies (none for an + * insert, the pre-image for a by-id update, the matched rows for a + * predicate update), projected by {@link storedReferencesCallerReads} — + * and then the key is dropped, as the echoed mask is. The read door serves + * a `secret` column masked ({@link maskSecretFields}), never as stored, so + * no stored ref is kept: every ref is REFUSED with * {@link SecretRefWriteError} (ADR-0112 `VALIDATION_ERROR`/400), one - * answer whatever the ref names. + * answer whatever the ref names and whatever the column holds. * - **Fail-closed:** any other secret value with no CryptoProvider registered, * or no reachable `sys_secret` store, THROWS — never persists cleartext. * (A `password` field needs no CryptoProvider — it is stored as-is.) */ export async function encryptSecretFields( - this: Pick, + this: Pick, object: string, row: Record, context: ExecutionContext | undefined, @@ -256,7 +271,18 @@ export async function encryptSecretFields( // predicate that matches nothing) has nothing to keep. Kept, the key is // dropped, as the echoed mask is: the column is not rewritten at all, so // the verdict cannot go stale before the statement runs. - ownRows ??= (await storedRows()) ?? []; + // ⛔ The rows are compared only as this caller's own read door serves + // them, never as stored: the answer to a column the caller cannot read + // as stored must not depend on what it holds. The door serves a + // `secret` column masked, so no ref survives the projection and every + // ref this caller sends is refused alike. + ownRows ??= await storedReferencesCallerReads.call( + this, + object, + ((await storedRows()) ?? []).map((own) => (own && typeof own === 'object' ? own : {})), + secretFields, + context, + ); const keepsOwn = ownRows.length > 0 && ownRows.every((own) => own?.[field] === value); if (!keepsOwn) throw new SecretRefWriteError(object, field); delete row[field]; diff --git a/packages/objectql/src/engine-validate.ts b/packages/objectql/src/engine-validate.ts index 4f77b46dd85..84a7cdf7b3e 100644 --- a/packages/objectql/src/engine-validate.ts +++ b/packages/objectql/src/engine-validate.ts @@ -442,7 +442,9 @@ export async function validate( // payload-FK-first reading (`masterIdOf`): the row's own FK on an insert, // and on an update the patch's FK, else the stored row's. A repointing // update also binds the header the stored row hangs off as - // `previousParent`, the header the write's ADR-0113 pre-check reads. + // `previousParent`, the header the write's ADR-0113 pre-check reads — and, + // as the write binds it, the unresolved header (`null`) when the patch + // names the FK over a stored FK this caller was not served as stored. // // ⛔ Read under the caller's access, as the write reads it since #22519: // see {@link resolvePreviewParents}. A header the caller cannot read binds diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 455ca65d6ca..308a367b07b 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -312,7 +312,7 @@ import { // hand each body the members it reads (`MasterDetailRelatedHost`). Imported after // every module those bodies import, so the engine's module evaluation order // is what it was. -import { resolveOneHopRelated, resolvePredicateRelated, resolvePreviewParents, settleMasterDetailLanding } from './engine-master-detail-related.js'; +import { priorMasterIdsCallerReads, resolveOneHopRelated, resolvePredicateRelated, resolvePreviewParents, settleMasterDetailLanding } from './engine-master-detail-related.js'; // [#25522 cut 15] The `ReadableIds` chain composition, the controlled-by-parent // boot screen, and the permission and formula resolutions an operation shares; // the methods of the same names below hand each body the members it reads @@ -2843,6 +2843,20 @@ export class ObjectQL implements IObjectQLEngine { return resolvePreviewParents.call(host, schema, views, storedRows, context); } + private async priorMasterIdsCallerReads( + schema: unknown, + rows: ReadonlyArray | null | undefined>, + context: ExecutionContext | undefined, + ): Promise> { + // The body lives in `engine-master-detail-related.ts`, its docblock with it (see `MasterDetailRelatedHost`). + const readLogger = () => this.logger; + const host: ThisParameterType = { + find: (...a) => this.find(...a), + get logger() { return readLogger(); }, + }; + return priorMasterIdsCallerReads.call(host, schema, rows, context); + } + private async resolvePredicateRelated( schema: any, rows: ReadonlyArray | undefined | null>, @@ -3109,10 +3123,14 @@ export class ObjectQL implements IObjectQLEngine { // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts`, its docblock with it (see `SecretInternalFieldsHost`). const readRegistry = () => this._registry; const readCryptoProvider = () => this.cryptoProvider; + const readLogger = () => this.logger; const host: ThisParameterType = { get _registry() { return readRegistry(); }, get cryptoProvider() { return readCryptoProvider(); }, + get logger() { return readLogger(); }, getDriver: (...a) => this.getDriver(...a), + // A ref on a non-system write is compared against its own rows as the caller's read door serves them. + find: (...a) => this.find(...a), }; return encryptSecretFields.call(host, object, row, context, driverOptions, storedRows); } @@ -7711,15 +7729,29 @@ export class ObjectQL implements IObjectQLEngine { // and `evaluateValidationRules` reuses `parent` for both. // [#19853] "Actually moves" is asked of the view the write // stores: a repoint that never lands moves nothing. + // ⛔ And of the stored FK only as the caller's own read door + // serves it (`priorMasterIdsCallerReads`), asked only when the + // write names the FK: a FK the caller cannot read as stored + // leaves the prior master UNKNOWN, bound as the unresolved + // header (`null`), so whether the pre-check reads a second + // header, and which, never depends on what the column holds. + // A FK the caller reads, and a system write, are judged as + // before. const mdRel = schemaHasParentRequiredWhen ? resolveMasterDetailRelation(updateSchema as any) : null; - const priorMasterId = mdRel ? masterIdOf(mdRel.fk, null, priorRecord) : undefined; + const landingView = landing?.view ?? preRoWhen; + const namesMaster = mdRel != null && landingView != null && typeof landingView === 'object' && mdRel.fk in landingView; + const [priorMaster] = namesMaster + ? await this.priorMasterIdsCallerReads(updateSchema, [priorRecord], opCtx.context) + : [undefined]; const repointsMaster = mdRel != null && - priorMasterId != null && - masterIdOf(mdRel.fk, landing?.view ?? preRoWhen, priorRecord) !== priorMasterId; - const roWhenPreviousParent = repointsMaster - ? await this.resolveMasterDetailParent(updateSchema, null, priorRecord, opCtx.context) - : undefined; + priorMaster?.id != null && + masterIdOf(mdRel.fk, landingView, null) !== priorMaster.id; + const roWhenPreviousParent = priorMaster === null + ? null + : repointsMaster + ? await this.resolveMasterDetailParent(updateSchema, null, { [mdRel!.fk]: priorMaster!.id }, opCtx.context) + : undefined; // [#9107] `suppliedValues` — the SAME entry snapshot the static // strip below consumes, now feeding the conditional one too, so // both strips answer "did the CALLER write this key?" from one @@ -8102,11 +8134,23 @@ export class ObjectQL implements IObjectQLEngine { // resolved only when the payload REPOINTS the matched rows at // another master (see the single-id branch for why the stored // row's own header is the one that question needs). + // ⛔ Each matched row's stored FK only as the caller's own read + // door serves it, as on the by-id branch: a row whose FK the + // caller cannot read as stored binds the unresolved header + // (`null`), and its stored master is never read. const mdRelMulti = schemaHasParentRequiredWhenMulti ? resolveMasterDetailRelation(updateSchema as any) : null; - const previousParentForRow = - mdRelMulti != null && masterIdOf(mdRelMulti.fk, landingMulti?.view ?? preRoWhenMulti, undefined) != null - ? await this.resolveMasterDetailParents(updateSchema, null, priorRows, opCtx.context) - : undefined; + let previousParentForRow: ((row: Record | undefined) => Record | null) | undefined; + if (mdRelMulti != null && masterIdOf(mdRelMulti.fk, landingMulti?.view ?? preRoWhenMulti, undefined) != null) { + const matched = priorRows ?? []; + const priorMasters = await this.priorMasterIdsCallerReads(updateSchema, matched, opCtx.context); + const known = matched.flatMap((row, i) => { + const prior = priorMasters[i]; + return prior ? [{ row, stored: { [mdRelMulti.fk]: prior.id } }] : []; + }); + const headerOf = await this.resolveMasterDetailParents(updateSchema, null, known.map((k) => k.stored), opCtx.context); + const previousByRow = new Map(known.map((k) => [k.row, headerOf(k.stored)])); + previousParentForRow = (row) => (row ? previousByRow.get(row) ?? null : null); + } if (payloadHasReadonlyWhen) { // [#9107] Same entry snapshot, same authorship gate as the // by-id branch above — "both call sites" is the #3106 / diff --git a/packages/objectql/src/secret-field-own-ref-write.test.ts b/packages/objectql/src/secret-field-own-ref-write.test.ts index 6b825ebe586..da5927b1ebd 100644 --- a/packages/objectql/src/secret-field-own-ref-write.test.ts +++ b/packages/objectql/src/secret-field-own-ref-write.test.ts @@ -2,15 +2,17 @@ /** * [#27582] A non-system write may only seal a new value or keep its own - * record's ref. + * record's ref — and "its own" is the value its caller reads in that column. * * A `secret` field's stored value is an opaque `secret:` ref. On a write that - * is not the system principal's, a value already spelled as a ref is accepted - * only when it is the ref that same record's same column already holds, and - * then it changes nothing (like the echoed read mask). Any other ref is - * refused with the ADR-0112 `VALIDATION_ERROR` / 400 pair, before any - * credential row or record is written, and with one answer whatever the ref - * names. A system write keeps the pass-through. + * is not the system principal's, a value already spelled as a ref is compared + * with the rows the write lands on only as the caller's own read door serves + * them, and the read door serves a `secret` column masked to every caller. So + * no ref is one such a write keeps: every ref is refused with the ADR-0112 + * `VALIDATION_ERROR` / 400 pair, before any credential row or record is + * written, with one answer whatever the ref names and whatever the column + * holds — the ref the row itself holds included. The echoed read mask is + * still a no-op, and a system write keeps the pass-through. * * Driven through a real {@link ObjectQL} over an in-memory stub driver, on * every engine write door the credential pass sits on: insert (single, batch @@ -122,7 +124,7 @@ const USER = { context: { isSystem: false, userId: 'u1' } } as any; /** A ref spelled like a stored one, naming no `sys_secret` row at all. */ const UNKNOWN_REF = makeSecretRef('sec_none'); -describe('[#27582] a non-system write may only seal a new value or keep its own record\'s ref', () => { +describe('[#27582] a non-system write may only seal a new value, never send a ref', () => { let engine: ObjectQL; let d: ReturnType; let crypto: ReturnType; @@ -210,22 +212,25 @@ describe('[#27582] a non-system write may only seal a new value or keep its own }); }); - describe('by-id update: only the ref this record already holds', () => { - it('keeping its own ref is accepted and changes nothing', async () => { + describe('by-id update: a ref is answered the same whatever the column holds', () => { + it('the ref this record holds is refused exactly as another record\'s, and nothing is written', async () => { const own = await seed('r1', 'mine'); - const before = secrets().map((s) => s.id); - await engine.update(VAULT, { id: 'r1', name: 'renamed', token: own }, USER); - expect(stored('r1')).toMatchObject({ name: 'renamed', token: own }); - // Kept, not rewritten: the statement leaves the column alone, as for the echoed mask. - expect(d.lastStatement(VAULT)).toMatchObject({ name: 'renamed' }); - expect(d.lastStatement(VAULT)).not.toHaveProperty('token'); - expect(secrets().map((s) => s.id)).toEqual(before); + const theirs = await seed('r2', 'theirs'); + const before = { secrets: secrets().map((s) => s.id), statement: d.lastStatement(VAULT) }; + const kept = await caught(engine.update(VAULT, { id: 'r1', name: 'renamed', token: own }, USER)); + const named = await caught(engine.update(VAULT, { id: 'r1', name: 'renamed', token: theirs }, USER)); + expect(kept).toMatchObject(refusal()); + expect(named).toMatchObject(refusal()); + expect(kept!.message).toBe(named!.message); + expect(stored('r1')).toMatchObject({ name: 'r1', token: own }); + expect(d.lastStatement(VAULT)).toBe(before.statement); + expect(secrets().map((s) => s.id)).toEqual(before.secrets); expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); }); - it('keeping its own ref as the only key is accepted too', async () => { + it('the ref this record holds, as the only key, is refused too', async () => { const own = await seed('r1', 'mine'); - await engine.update(VAULT, { id: 'r1', token: own }, USER); + expect(await caught(engine.update(VAULT, { id: 'r1', token: own }, USER))).toMatchObject(refusal()); expect(refOf('r1')).toBe(own); expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); }); @@ -236,11 +241,21 @@ describe('[#27582] a non-system write may only seal a new value or keep its own expect(refOf('r1')).toBe(own); }); - it('a beforeUpdate hook that writes the stored row back keeps its own ref', async () => { + it('a beforeUpdate hook that writes the stored row back is judged on the ref it carries, as a caller is', async () => { const own = await seed('r1', 'mine'); engine.registerHook('beforeUpdate', (ctx: any) => { ctx.input.data = { ...ctx.previous, ...ctx.input.data }; }, { object: VAULT }); + expect(await caught(engine.update(VAULT, { id: 'r1', name: 'renamed' }, USER))).toMatchObject(refusal()); + expect(stored('r1')).toMatchObject({ name: 'r1', token: own }); + }); + + it('a beforeUpdate hook that writes the stored row back without its secret column lands', async () => { + const own = await seed('r1', 'mine'); + engine.registerHook('beforeUpdate', (ctx: any) => { + const { token: _held, ...rest } = ctx.previous; + ctx.input.data = { ...rest, ...ctx.input.data }; + }, { object: VAULT }); await engine.update(VAULT, { id: 'r1', name: 'renamed' }, USER); expect(stored('r1')).toMatchObject({ name: 'renamed', token: own }); expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); @@ -282,18 +297,21 @@ describe('[#27582] a non-system write may only seal a new value or keep its own }); }); - describe('predicate update: only a ref every matched record already holds', () => { + describe('predicate update: a ref is answered the same whatever the matched rows hold', () => { const multi = (where: Row) => ({ ...USER, where, multi: true }); - it('a ref every matched row already holds is accepted and changes nothing', async () => { + it('a ref every matched row holds is refused exactly as one none holds, and nothing is written', async () => { await seed('r1', 'one'); await seed('r2', 'two'); await engine.update(VAULT, { token: 'shared' }, { ...SYSTEM, where: { tier: 'gold' }, multi: true }); const shared = refOf('r1'); expect(refOf('r2')).toBe(shared); - await engine.update(VAULT, { name: 'renamed', token: shared }, multi({ tier: 'gold' })); - expect(d.rowsOf(VAULT).map((r) => [r.name, r.token])).toEqual([['renamed', shared], ['renamed', shared]]); - expect(d.lastStatement(VAULT)).not.toHaveProperty('token'); + const held = await caught(engine.update(VAULT, { name: 'renamed', token: shared }, multi({ tier: 'gold' }))); + const unknown = await caught(engine.update(VAULT, { name: 'renamed', token: UNKNOWN_REF }, multi({ tier: 'gold' }))); + expect(held).toMatchObject(refusal()); + expect(unknown).toMatchObject(refusal()); + expect(held!.message).toBe(unknown!.message); + expect(d.rowsOf(VAULT).map((r) => [r.name, r.token])).toEqual([['r1', shared], ['r2', shared]]); expect(await engine.resolveSecretField(VAULT, 'r2', 'token')).toBe('shared'); }); diff --git a/packages/objectql/src/validate-preview-parent.test.ts b/packages/objectql/src/validate-preview-parent.test.ts index c69478c8865..983c0dbbd83 100644 --- a/packages/objectql/src/validate-preview-parent.test.ts +++ b/packages/objectql/src/validate-preview-parent.test.ts @@ -42,6 +42,10 @@ // writes in `engine.ts`, the preview in `engine-validate.ts`). // // (a), (b) and (e) were red before the fix. +// +// A later describe pins the repoint over a stored FK the caller is not served +// as stored: the preview binds the prior header as unresolved, as the write +// does, so both give one verdict whatever header the row hung off. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { readFileSync } from 'node:fs'; @@ -382,6 +386,49 @@ describe('#22474 — the preview binds the master-detail header the write binds' }); }); +describe('a repoint over a stored FK the caller is not served: the preview binds the prior header as the write does', () => { + /** + * A field-level read rule on the line's `invoice` FK for one caller, shaped + * like the security middleware's result mask (`hide` deletes the key). The + * write's projection of the stored FK and the preview's stored-row read both + * go through the read door, so both meet it. + */ + function hideInvoiceFrom(ql: ObjectQL, userId: string) { + ql.registerMiddleware(async (ctx: any, next: () => Promise) => { + await next(); + if (ctx.object !== LINE || ctx.context?.isSystem || ctx.context?.userId !== userId) return; + if (!['find', 'findOne'].includes(ctx.operation)) return; + const apply = (r: any) => { if (r && typeof r === 'object') delete r.invoice; }; + if (Array.isArray(ctx.result)) ctx.result.forEach(apply); + else apply(ctx.result); + }); + } + + /** The preview's verdict and then the write's, for a line lacking its description that hangs off `stored`. */ + async function bothVerdicts(stored: string, context: Record) { + const { engine, storeFor } = makeEngine(); + hideInvoiceFrom(engine, 'frank'); + storeFor(LINE).set('l1', { id: 'l1', name: 'l', invoice: stored, quantity: 1 }); + const patch = { invoice: 'h_sent', id: 'l1' }; + const previewed = await preview(engine, LINE, patch, 'update', context); + const written = await write(() => engine.update(LINE, patch, { context } as any)); + return { previewed, written }; + } + + it('a caller not served the FK gets one verdict whatever header the line hangs off, in the preview and in the write', async () => { + const frank = { userId: 'frank' }; + for (const stored of ['h_sent', 'h_draft']) { + expect(await bothVerdicts(stored, frank)).toEqual({ previewed: REQUIRED, written: REQUIRED }); + } + }); + + it('control: a caller served the FK keeps the legacy rest and the repoint refusal, in both', async () => { + const alice = { userId: 'alice' }; + expect(await bothVerdicts('h_sent', alice)).toEqual({ previewed: ADMITTED, written: ADMITTED }); + expect(await bothVerdicts('h_draft', alice)).toEqual({ previewed: REQUIRED, written: REQUIRED }); + }); +}); + describe('#22474 (e) — the import dry run of a detail row, through the protocol', () => { let kernel: ObjectKernel; let objectql: ObjectQL; diff --git a/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts b/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts index 095dfde0adf..40c98d03e04 100644 --- a/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts +++ b/packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts @@ -31,10 +31,15 @@ * who can read the column keeps the forgiveness (the stored value carried * back lands) and is refused a new unreadable value; a system write keeps * its exemption. + * + * Two more update seams that compare a sent value with the stored row are + * pinned on the same rule at the end of this file: the stored header a + * parent-scoped `requiredWhen` pre-check reads, and the `secret` reference a + * write may keep. */ import { describe, it, expect, vi } from 'vitest'; -import { ObjectQL } from '@objectstack/objectql'; +import { ObjectQL, SECRET_MASK } from '@objectstack/objectql'; import { resolveThrownHttpError } from '@objectstack/types'; import type { PermissionSet } from '@objectstack/spec/security'; import { SecurityPlugin } from './security-plugin.js'; @@ -267,3 +272,221 @@ describe('the update\'s reference forgiveness compares only stored values the ca expect(storedItems(storeFor).find((r) => r.id === 'k1')?.duty).toBe(OTHER.duty); }); }); + +/** + * The same rule on the two other update seams that compare a sent value with + * the stored row: the master-detail header the ADR-0113 pre-check of a + * parent-scoped `requiredWhen` reads, and the `secret` reference a write may + * keep. Each is answered from the stored row only as the caller's own read + * door serves it, so for a column the caller may edit but not read the answer + * is the same whatever the column holds. + * + * The rig is the one above with its own objects: a header the caller can + * read, a detail line whose `header` FK carries the grant's `readable` bit, + * and a vault whose `token` secret does. Rows are seeded straight into the + * store, so a line can hold a stored state no write would produce (a legacy + * line that already lacks its required note). + */ +const HEADER = 'rfc_header'; +const LINE = 'rfc_line'; +const VAULT = 'rfc_vault'; + +const HEADER_OBJECT = { name: HEADER, fields: { name: { type: 'text' }, status: { type: 'text' } } }; +const LINE_OBJECT = { + name: LINE, + fields: { + title: { type: 'text' }, + header: { type: 'master_detail', reference: HEADER }, + note: { type: 'text', requiredWhen: "parent.status == 'paid'" }, + }, +}; +const SYS_SECRET_OBJECT = { + name: 'sys_secret', systemFields: { tenant: false }, + fields: { + namespace: { type: 'text' }, key: { type: 'text' }, kms_key_id: { type: 'text' }, alg: { type: 'text' }, + version: { type: 'number' }, ciphertext: { type: 'text' }, created_at: { type: 'datetime' }, + }, +}; +const VAULT_OBJECT = { name: VAULT, fields: { name: { type: 'text' }, token: { type: 'secret' } } }; + +/** Reads headers; edits lines and vaults; `readable` is the one bit two grants differ in. */ +function columnGrant(readable: boolean): PermissionSet { + return { + name: 'member_default', + label: readable ? 'Editor who may read the column' : 'Editor who may edit but not read the column', + objects: { + [HEADER]: { allowRead: true }, + [LINE]: { allowRead: true, allowCreate: true, allowEdit: true }, + [VAULT]: { allowRead: true, allowCreate: true, allowEdit: true }, + }, + fields: { + [`${LINE}.header`]: { readable, editable: true }, + [`${VAULT}.token`]: { readable, editable: true }, + }, + } as unknown as PermissionSet; +} + +function makeCrypto() { + let n = 0; + return { + async encrypt(plain: string) { + n += 1; + return { id: `sec_${n}`, kmsKeyId: 'local', alg: 'test-b64', version: 1, ciphertext: Buffer.from(plain).toString('base64') }; + }, + async decrypt(handle: { ciphertext: string }) { return Buffer.from(handle.ciphertext, 'base64').toString('utf8'); }, + async rotateKey(handle: unknown) { return handle; }, + digest(plain: string) { return `d:${plain.length}`; }, + async keyedDigest(plain: string) { return `k:${plain.length}`; }, + }; +} + +async function bootStored(set: PermissionSet) { + const engine = new ObjectQL(); + const d = makeDriver(); + engine.registerDriver(d.driver, true); + await engine.init(); + for (const object of [HEADER_OBJECT, LINE_OBJECT, SYS_SECRET_OBJECT, VAULT_OBJECT]) { + engine.registry.registerObject(object as any, 'test-package'); + } + engine.setCryptoProvider(makeCrypto() as never); + d.storeFor(HEADER).set('h_paid', { id: 'h_paid', name: 'paid', status: 'paid' }); + d.storeFor(HEADER).set('h_open', { id: 'h_open', name: 'open', status: 'open' }); + + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [set], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as any); + await plugin.start(ctx as any); + return { engine, storeFor: d.storeFor }; +} + +/** The answer a write gets, whole: `ok`, or the status, code, sentence and field findings (ADR-0112). */ +async function outcomeOf(run: () => Promise): Promise { + try { + await run(); + return 'ok'; + } catch (err) { + const thrown = resolveThrownHttpError(err); + return JSON.stringify({ + status: thrown.status, + code: thrown.code, + message: (err as Error).message, + fields: ((err as any)?.fields ?? []).map((f: any) => ({ field: f.field, code: f.code, message: f.message })), + }); + } +} + +describe('a parent-scoped requiredWhen pre-check reads the stored header only through a FK the caller may read', () => { + /** Two lines that already lack the note, each hanging off `stored`. */ + async function rig(readable: boolean, stored: 'h_paid' | 'h_open') { + const booted = await bootStored(columnGrant(readable)); + for (const id of ['l1', 'l2']) booted.storeFor(LINE).set(id, { id, title: 'stored', header: stored, note: null }); + return booted; + } + const writeLines = (engine: ObjectQL, door: Door, payload: Record, context: Record = CALLER) => + door === 'update by id' + ? engine.update(LINE, payload, { where: { id: 'l1' }, context } as any) + : engine.update(LINE, payload, { where: { id: { $in: ['l1', 'l2'] } }, multi: true, context } as any); + const linesOf = (storeFor: (o: string) => Map) => + [...storeFor(LINE).values()].map((r) => ({ id: r.id, title: r.title, header: r.header })); + + it.each(DOORS)('%s — naming a header with the FK hidden answers the same whatever the line stores', async (door) => { + const answers: string[] = []; + for (const stored of ['h_paid', 'h_open'] as const) { + const { engine, storeFor } = await rig(false, stored); + answers.push(await outcomeOf(() => writeLines(engine, door, { title: 'renamed', header: 'h_paid' }))); + // Refused before the statement: the lines are as stored. + expect(linesOf(storeFor)).toEqual(['l1', 'l2'].map((id) => ({ id, title: 'stored', header: stored }))); + } + expect(answers[0]).toBe(answers[1]); + const answer = JSON.parse(answers[0]); + expect({ status: answer.status, code: answer.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' }); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field: 'note', code: 'required' }]); + }); + + it.each(DOORS)('%s — control: with the FK hidden, an edit that does not name it is judged as before', async (door) => { + const { engine, storeFor } = await rig(false, 'h_paid'); + await writeLines(engine, door, { title: 'renamed' }); + expect(linesOf(storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'renamed', header: 'h_paid' }); + }); + + it.each(DOORS)('%s — control: a caller who reads the FK keeps the legacy rest, and a repoint is judged as before', async (door) => { + const resting = await rig(true, 'h_paid'); + await writeLines(resting.engine, door, { title: 'renamed', header: 'h_paid' }); + expect(linesOf(resting.storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'renamed', header: 'h_paid' }); + + const repointed = await rig(true, 'h_open'); + const answer = JSON.parse(await outcomeOf(() => writeLines(repointed.engine, door, { title: 'renamed', header: 'h_paid' }))); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field: 'note', code: 'required' }]); + expect(linesOf(repointed.storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'stored', header: 'h_open' }); + }); + + it.each(DOORS)('%s — control: a system write is judged on the stored row as before', async (door) => { + const resting = await rig(false, 'h_paid'); + await writeLines(resting.engine, door, { title: 'renamed', header: 'h_paid' }, SYS_CTX); + expect(linesOf(resting.storeFor).find((r) => r.id === 'l1')).toEqual({ id: 'l1', title: 'renamed', header: 'h_paid' }); + + const repointed = await rig(false, 'h_open'); + const answer = JSON.parse(await outcomeOf(() => writeLines(repointed.engine, door, { title: 'renamed', header: 'h_paid' }, SYS_CTX))); + expect(answer.fields.map((f: any) => ({ field: f.field, code: f.code }))).toEqual([{ field: 'note', code: 'required' }]); + }); +}); + +describe('a secret reference on a non-system update is answered the same whatever the column holds', () => { + /** Two vault rows sealed by a system write; `v1` and `v2` hold different refs. */ + async function rig(readable: boolean) { + const booted = await bootStored(columnGrant(readable)); + for (const id of ['v1', 'v2']) await booted.engine.insert(VAULT, { id, name: id, token: `${id}-value` }, { context: SYS_CTX } as any); + const refOf = (id: string) => booted.storeFor(VAULT).get(id)?.token as string; + return { ...booted, refOf }; + } + const writeVault = (engine: ObjectQL, door: Door, payload: Record, context: Record = CALLER) => + door === 'update by id' + ? engine.update(VAULT, payload, { where: { id: 'v1' }, context } as any) + : engine.update(VAULT, payload, { where: { id: { $in: ['v1'] } }, multi: true, context } as any); + + describe.each([false, true])('the caller reads the column: %s', (readable) => { + it.each(DOORS)('%s — the ref the row holds, another row\'s and one naming nothing get one answer', async (door) => { + const { engine, storeFor, refOf } = await rig(readable); + const before = [...storeFor(VAULT).values()].map((r) => ({ ...r })); + const own = await outcomeOf(() => writeVault(engine, door, { name: 'renamed', token: refOf('v1') })); + const other = await outcomeOf(() => writeVault(engine, door, { name: 'renamed', token: refOf('v2') })); + const none = await outcomeOf(() => writeVault(engine, door, { name: 'renamed', token: 'secret:sec_none' })); + expect(own).toBe(other); + expect(own).toBe(none); + expect(JSON.parse(own)).toMatchObject({ status: 400, code: 'VALIDATION_ERROR' }); + expect([...storeFor(VAULT).values()]).toEqual(before); + }); + + it.each(DOORS)('%s — control: a new value is sealed, and the echoed mask leaves the column alone', async (door) => { + const { engine, storeFor, refOf } = await rig(readable); + const held = refOf('v1'); + await writeVault(engine, door, { name: 'masked', token: SECRET_MASK }); + expect(storeFor(VAULT).get('v1')).toMatchObject({ name: 'masked', token: held }); + await writeVault(engine, door, { token: 'v1-next' }); + expect(refOf('v1')).toMatch(/^secret:/); + expect(refOf('v1')).not.toBe(held); + expect(await engine.resolveSecretField(VAULT, 'v1', 'token')).toBe('v1-next'); + }); + }); + + it.each(DOORS)('%s — control: a system write keeps the pass-through', async (door) => { + const { engine, refOf } = await rig(false); + await writeVault(engine, door, { token: refOf('v2') }, SYS_CTX); + expect(refOf('v1')).toBe(refOf('v2')); + }); +}); diff --git a/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts b/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts index 8371108c030..7fe3bb03310 100644 --- a/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts +++ b/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts @@ -1,7 +1,9 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // // [#27582] A non-system write may only seal a new value or keep its own -// record's ref — pinned at the REST data door of a booted stack with a crypto +// record's ref, and its own is the value its caller reads in that column — a +// `secret` column reads masked, so no ref is one it keeps. Pinned at the REST +// data door of a booted stack with a crypto // provider wired (`bootStack` wires one LocalCryptoProvider, as // `objectstack dev` / `serve` do). Every HTTP request is assembled as a // non-system caller. `packages/objectql/src/secret-field-own-ref-write.test.ts` @@ -10,10 +12,11 @@ // // - a create whose `secret` field holds a ref is refused (`VALIDATION_ERROR` / // 400), on the single-record door, the bulk door and the import door; -// - an update that keeps the record's own value is accepted and changes -// nothing — the read mask the form echoes, and the record's own ref; -// - an update naming any other ref is refused with one answer, whatever that -// ref names, and the stored value stays; +// - an update that echoes the read mask the form shows is accepted and +// changes nothing; +// - an update naming any ref is refused with one answer, whatever that ref +// names and whatever the column holds (the record's own ref included), and +// the stored value stays; // - the system principal keeps the pass-through. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -42,7 +45,7 @@ const sorStack = defineStack({ type Answer = { status: number; json: any; text: string }; -describe('[#27582] secret field: a non-system write may only seal a new value or keep its own record\'s ref — over REST', () => { +describe('[#27582] secret field: a non-system write may only seal a new value, never send a ref — over REST', () => { let stack: VerifyStack; let ql: any; let token = ''; @@ -109,7 +112,7 @@ describe('[#27582] secret field: a non-system write may only seal a new value or expect(await stack.rows(VAULT, { name: 'sor-import' })).toHaveLength(0); }); - it('an update that echoes the read mask, or the record\'s own ref, is accepted and changes nothing', async () => { + it('an update that echoes the read mask is accepted and changes nothing', async () => { const id = await create('sor-own', 'sor-own-value'); const own = String(await storedRef(id)); const before = await secretRowCount(); @@ -120,16 +123,13 @@ describe('[#27582] secret field: a non-system write may only seal a new value or const masked = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-own-renamed', token: echoed }); expect(masked.status, masked.text).toBe(200); - const kept = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-own-again', token: own }); - expect(kept.status, kept.text).toBe(200); - expect(await storedRef(id)).toBe(own); expect(await secretRowCount()).toBe(before); expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-own-value'); - expect((await stack.rows(VAULT, { id }))[0].name).toBe('sor-own-again'); + expect((await stack.rows(VAULT, { id }))[0].name).toBe('sor-own-renamed'); }); - it('an update naming any other ref is refused with one answer, and the stored value stays', async () => { + it('an update naming any ref — the record\'s own included — is refused with one answer, and the stored value stays', async () => { const id = await create('sor-keep', 'sor-keep-value'); const own = String(await storedRef(id)); const before = await secretRowCount(); @@ -142,6 +142,9 @@ describe('[#27582] secret field: a non-system write may only seal a new value or const unknown = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-keep-renamed', token: unknownRef }); expect(answerOf(unknown)).toEqual(answerOf(named)); + const kept = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-keep-renamed', token: own }); + expect(answerOf(kept)).toEqual(answerOf(named)); + expect(await storedRef(id)).toBe(own); expect(await secretRowCount()).toBe(before); expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-keep-value');